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

Merge pull request #22784 from edx/BOM-1139

Browse Source 
Replace deprecated password_reset_confirm with class based view - BOM-1139
This commit is contained in:
Aarif
2020-01-30 12:48:28 +05:00
committed by GitHub
parent 0a1056f001 2827f545b1
commit 0cb0867cd7
3 changed files with 207 additions and 136 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
openedx/core/djangoapps/user_authn/urls_common.py
View File

@@ -13,10 +13,10 @@ which leads to inconsistent prefixing.
from django.conf import settings
from django.conf.urls import url
from django.contrib.auth.views import password_reset_complete
from django.contrib.auth.views import PasswordResetCompleteView
from .views import auto_auth, login, logout, password_reset, register
from .views.password_reset import PasswordResetConfirmWrapper
urlpatterns = [
# Registration
@@ -55,7 +55,7 @@ urlpatterns = [
url(r'^password_reset/$', password_reset.password_reset, name='password_reset'),
url(
r'^password_reset_confirm/(?P<uidb36>[0-9A-Za-z]+)-(?P<token>.+)/$',
password_reset.password_reset_confirm_wrapper,
PasswordResetConfirmWrapper.as_view(),
name='password_reset_confirm',
),
url(r'^account/password$', password_reset.password_change_request_handler, name='password_change_request'),
@@ -66,7 +66,7 @@ urlpatterns = [
urlpatterns += [
url(
r'^password_reset_complete/$',
password_reset_complete,
PasswordResetCompleteView.as_view(),
name='password_reset_complete',
),
]

292
openedx/core/djangoapps/user_authn/views/password_reset.py
View File

@@ -1,19 +1,21 @@
""" Password reset logic and views . """
import logging
from django import forms
from django.contrib.auth.forms import PasswordResetForm
from django.contrib.auth.forms import PasswordResetForm, SetPasswordForm
from django.conf import settings
from django.contrib import messages
from django.contrib.auth.hashers import UNUSABLE_PASSWORD_PREFIX
from django.contrib.auth.models import User
from django.contrib.auth.tokens import default_token_generator
from django.contrib.auth.views import password_reset_confirm
from django.contrib.auth.views import (
INTERNAL_RESET_SESSION_TOKEN, PasswordResetConfirmView)
from django.core.exceptions import ObjectDoesNotExist
from django.core.validators import ValidationError
from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseForbidden
from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseForbidden, HttpResponseRedirect
from django.template.response import TemplateResponse
from django.utils.decorators import method_decorator
from django.utils.encoding import force_bytes, force_text
@@ -50,7 +52,6 @@ from util.json_request import JsonResponse
from util.password_policy_validators import normalize_password, validate_password
from util.request_rate_limiter import BadRequestRateLimiter, PasswordResetEmailRateLimiter
SETTING_CHANGE_INITIATED = 'edx.user.settings.change_initiated'
# Maintaining this naming for backwards compatibility.
@@ -159,7 +160,7 @@ class PasswordResetFormNoActive(PasswordResetForm):
Validates that a user exists with the given email address.
"""
email = self.cleaned_data["email"]
#The line below contains the only change, removing is_active=True
# The line below contains the only change, removing is_active=True
self.users_cache = User.objects.filter(email__iexact=email)
if not self.users_cache and is_secondary_email_feature_enabled():
@@ -299,62 +300,158 @@ def _uidb36_to_uidb64(uidb36):
return uidb64
# pylint: disable=too-many-statements
def password_reset_confirm_wrapper(request, uidb36=None, token=None):
class PasswordResetConfirmWrapper(PasswordResetConfirmView):
"""
A wrapper around django.contrib.auth.views.password_reset_confirm.
Needed because we want to set the user as active at this step.
We also optionally do some additional password policy checks.
A wrapper around django.contrib.auth.views.PasswordResetConfirmView.
Needed because we want to set the user as active at this step.
We also optionally do some additional password policy checks.
"""
# convert old-style base36-encoded user id to base64
uidb64 = _uidb36_to_uidb64(uidb36)
platform_name = {
"platform_name": configuration_helpers.get_value('platform_name', settings.PLATFORM_NAME)
}
# User can not get this link unless account recovery feature is enabled.
if 'is_account_recovery' in request.GET and not is_secondary_email_feature_enabled():
raise Http404
reset_url_token = 'set-password'
try:
uid_int = base36_to_int(uidb36)
if request.user.is_authenticated and request.user.id != uid_int:
raise Http404
def __init__(self):
self.post_reset_login = False
self.platform_name = PasswordResetConfirmWrapper._get_platform_name()
self.validlink = False
self.user = None
self.uidb36 = ''
self.token = ''
self.uidb64 = ''
self.uid_int = -1
user = User.objects.get(id=uid_int)
except (ValueError, User.DoesNotExist):
# if there's any error getting a user, just let django's
# password_reset_confirm function handle it.
return password_reset_confirm(
request, uidb64=uidb64, token=token, extra_context=platform_name
)
def _process_password_reset_success(self, request, token, uidb64, extra_context):
self.user = self.get_user(uidb64)
form = SetPasswordForm(self.user, request.POST)
if self.token_generator.check_token(self.user, token) and form.is_valid():
self.form_valid(form)
url = reverse('password_reset_complete')
return HttpResponseRedirect(url)
else:
context = self.get_context_data()
if extra_context is not None:
context.update(extra_context)
return self.render_to_response(context)
def _set_token_in_session(self, request, token):
"""
method to store password reset token in session received in reset password url
"""
if not token:
return
session = request.session
session[INTERNAL_RESET_SESSION_TOKEN] = token
session.save()
@staticmethod
def _get_platform_name():
return {"platform_name": configuration_helpers.get_value('platform_name', settings.PLATFORM_NAME)}
def _set_user(self, request):
try:
self.uid_int = base36_to_int(self.uidb36)
if request.user.is_authenticated and request.user.id != self.uid_int:
raise Http404
self.user = User.objects.get(id=self.uid_int)
except (ValueError, User.DoesNotExist):
# if there's any error getting a user, just let django's
# password_reset_confirm function handle it.
return super(PasswordResetConfirmWrapper, self).dispatch(request, uidb64=self.uidb64, token=self.token,
extra_context=self.platform_name)
def _handle_retired_user(self, request):
"""
method responsible to stop password reset in case user is retired
"""
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)
context.update(self.platform_name)
return TemplateResponse(
request, 'registration/password_reset_confirm.html', context
)
if waffle().is_enabled(PREVENT_AUTH_USER_WRITES):
def _handle_system_unavailability(self, request):
"""
method to stop password reset process if system is under maintenance
"""
context = {
'validlink': False,
'form': None,
'title': _('Password reset unsuccessful'),
'err_msg': SYSTEM_MAINTENANCE_MSG,
}
context.update(platform_name)
context.update(self.platform_name)
return TemplateResponse(
request, 'registration/password_reset_confirm.html', context
)
if request.method == 'POST':
def _validate_password(self, password, request):
try:
validate_password(password, user=self.user)
except ValidationError as err:
context = {
'validlink': True,
'form': None,
'title': _('Password reset unsuccessful'),
'err_msg': ' '.join(err.messages),
}
context.update(self.platform_name)
return TemplateResponse(
request, 'registration/password_reset_confirm.html', context
)
def _handle_password_reset_failure(self, response):
form_valid = response.context_data['form'].is_valid() if response.context_data['form'] else False
if not form_valid:
log.warning(
u'Unable to reset password for user [%s] because form is not valid. '
u'A possible cause is that the user had an invalid reset token',
self.user.username,
)
response.context_data['err_msg'] = _('Error in resetting your password. Please try again.')
return response
def _handle_primary_email_update(self, updated_user):
try:
updated_user.email = updated_user.account_recovery.secondary_email
updated_user.account_recovery.delete()
# emit an event that the user changed their secondary email to the primary email
tracker.emit(
SETTING_CHANGE_INITIATED,
{
"setting": "email",
"old": self.user.email,
"new": updated_user.email,
"user_id": updated_user.id,
}
)
except ObjectDoesNotExist:
log.error('Account recovery process initiated without AccountRecovery instance for user {username}'
.format(username=updated_user.username))
def _handle_password_creation(self, request, updated_user):
messages.success(
request,
HTML(_(
u'{html_start}Password Creation Complete{html_end}'
u'Your password has been created. {bold_start}{email}{bold_end} is now your primary login email.'
)).format(
support_url=configuration_helpers.get_value('SUPPORT_SITE_LINK', settings.SUPPORT_SITE_LINK),
html_start=HTML('<p class="message-title">'),
html_end=HTML('</p>'),
bold_start=HTML('<b>'),
bold_end=HTML('</b>'),
email=updated_user.email,
),
extra_tags='account-recovery aa-icon submission-success'
)
def post(self, request, *args, **kwargs):
# We have to make a copy of request.POST because it is a QueryDict object which is immutable until copied.
# We have to use request.POST because the password_reset_confirm method takes in the request and a user's
# password is set to the request.POST['new_password1'] field. We have to also normalize the new_password2
@@ -366,104 +463,63 @@ def password_reset_confirm_wrapper(request, uidb36=None, token=None):
request.POST['new_password2'] = normalize_password(request.POST['new_password2'])
password = request.POST['new_password1']
try:
validate_password(password, user=user)
except ValidationError as err:
# We have a password reset attempt which violates some security
# policy, or any other validation. Use the existing Django template to communicate that
# back to the user.
context = {
'validlink': True,
'form': None,
'title': _('Password reset unsuccessful'),
'err_msg': ' '.join(err.messages),
}
context.update(platform_name)
return TemplateResponse(
request, 'registration/password_reset_confirm.html', context
)
response = self._validate_password(password, request)
if response:
return response
if 'is_account_recovery' in request.GET:
response = password_reset_confirm(
request,
uidb64=uidb64,
token=token,
extra_context=platform_name,
template_name='registration/password_reset_confirm.html',
post_reset_redirect='signin_user',
)
self.post_reset_login = True
response = self._process_password_reset_success(request, self.token, self.uidb64,
extra_context=self.platform_name)
else:
response = password_reset_confirm(
request, uidb64=uidb64, token=token, extra_context=platform_name
)
response = self._process_password_reset_success(request, self.token, self.uidb64,
extra_context=self.platform_name)
# If password reset was unsuccessful a template response is returned (status_code 200).
# Check if form is invalid then show an error to the user.
# Note if password reset was successful we get response redirect (status_code 302).
if response.status_code == 200:
form_valid = response.context_data['form'].is_valid() if response.context_data['form'] else False
if not form_valid:
log.warning(
u'Unable to reset password for user [%s] because form is not valid. '
u'A possible cause is that the user had an invalid reset token',
user.username,
)
response.context_data['err_msg'] = _('Error in resetting your password. Please try again.')
return response
return self._handle_password_reset_failure(response)
# get the updated user
updated_user = User.objects.get(id=uid_int)
updated_user = User.objects.get(id=self.uid_int)
if 'is_account_recovery' in request.GET:
try:
updated_user.email = updated_user.account_recovery.secondary_email
updated_user.account_recovery.delete()
# emit an event that the user changed their secondary email to the primary email
tracker.emit(
SETTING_CHANGE_INITIATED,
{
"setting": "email",
"old": user.email,
"new": updated_user.email,
"user_id": updated_user.id,
}
)
except ObjectDoesNotExist:
log.error(
u'Account recovery process initiated without AccountRecovery instance for user {username}'.format(
username=updated_user.username
)
)
self._handle_primary_email_update(updated_user)
updated_user.save()
if response.status_code == 302 and 'is_account_recovery' in request.GET:
messages.success(
request,
HTML(_(
u'{html_start}Password Creation Complete{html_end}'
u'Your password has been created. {bold_start}{email}{bold_end} is now your primary login email.'
)).format(
support_url=configuration_helpers.get_value('SUPPORT_SITE_LINK', settings.SUPPORT_SITE_LINK),
html_start=HTML('<p class="message-title">'),
html_end=HTML('</p>'),
bold_start=HTML('<b>'),
bold_end=HTML('</b>'),
email=updated_user.email,
),
extra_tags='account-recovery aa-icon submission-success'
)
else:
response = password_reset_confirm(
request, uidb64=uidb64, token=token, extra_context=platform_name
)
self._handle_password_creation(request, updated_user)
response_was_successful = response.context_data.get('validlink')
if response_was_successful and not user.is_active:
user.is_active = True
user.save()
return response
return response
def dispatch(self, *args, **kwargs):
self.uidb36 = kwargs.get('uidb36')
self.token = kwargs.get('token')
self.uidb64 = _uidb36_to_uidb64(self.uidb36)
# User can not get this link unless account recovery feature is enabled.
if 'is_account_recovery' in self.request.GET and not is_secondary_email_feature_enabled():
raise Http404
response = self._set_user(self.request)
if response:
return response
if UserRetirementRequest.has_user_requested_retirement(self.user):
return self._handle_retired_user(self.request)
if waffle().is_enabled(PREVENT_AUTH_USER_WRITES):
return self._handle_system_unavailability(self.request)
if self.request.method == 'POST':
return self.post(self.request, *args, **kwargs)
else:
self._set_token_in_session(self.request, self.token)
token = self.reset_url_token
response = super(PasswordResetConfirmWrapper, self).dispatch(self.request, uidb64=self.uidb64, token=token,
extra_context=self.platform_name)
response_was_successful = response.context_data.get('validlink')
if response_was_successful and not self.user.is_active:
self.user.is_active = True
self.user.save()
return response
def _get_user_from_email(email):

43
openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py
View File

@@ -13,6 +13,8 @@ from django.conf import settings
from django.contrib.auth.hashers import UNUSABLE_PASSWORD_PREFIX, make_password
from django.contrib.auth.models import AnonymousUser, User
from django.contrib.auth.tokens import default_token_generator
from django.contrib.auth.views import INTERNAL_RESET_SESSION_TOKEN, PasswordResetConfirmView
from django.contrib.sessions.middleware import SessionMiddleware
from django.core import mail
from django.core.cache import cache
from django.http import Http404
@@ -34,8 +36,8 @@ from openedx.core.djangoapps.user_api.models import UserRetirementRequest
from openedx.core.djangoapps.user_api.tests.test_views import UserAPITestCase
from openedx.core.djangoapps.user_api.accounts import EMAIL_MAX_LENGTH, EMAIL_MIN_LENGTH
from openedx.core.djangoapps.user_authn.views.password_reset import (
SETTING_CHANGE_INITIATED, password_reset, password_reset_confirm_wrapper
)
SETTING_CHANGE_INITIATED, password_reset,
PasswordResetConfirmWrapper)
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase
from student.tests.factories import UserFactory
from student.tests.test_configuration_overrides import fake_get_value
@@ -45,6 +47,12 @@ from util.password_policy_validators import create_validator_config
from util.testing import EventTestMixin
def process_request(request):
middleware = SessionMiddleware()
middleware.process_request(request)
request.session.save()
@unittest.skipUnless(
settings.ROOT_URLCONF == "lms.urls",
"reset password tests should only run in LMS"
@@ -55,7 +63,6 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
Tests that clicking reset password sends email, and doesn't activate the user
"""
request_factory = RequestFactory()
ENABLED_CACHES = ['default']
def setUp(self): # pylint: disable=arguments-differ
@@ -303,8 +310,9 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
kwargs={"uidb36": uidb36, "token": token}
)
)
process_request(bad_request)
bad_request.user = AnonymousUser()
password_reset_confirm_wrapper(bad_request, uidb36, token)
PasswordResetConfirmWrapper.as_view()(bad_request, uidb36=uidb36, token=token)
self.user = User.objects.get(pk=self.user.pk)
self.assertFalse(self.user.is_active)
@@ -317,8 +325,9 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
kwargs={"uidb36": self.uidb36, "token": self.token}
)
good_reset_req = self.request_factory.get(url)
process_request(good_reset_req)
good_reset_req.user = self.user
password_reset_confirm_wrapper(good_reset_req, self.uidb36, self.token)
PasswordResetConfirmWrapper.as_view()(good_reset_req, uidb36=self.uidb36, token=self.token)
self.user = User.objects.get(pk=self.user.pk)
self.assertTrue(self.user.is_active)
@@ -331,8 +340,9 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
kwargs={"uidb36": self.uidb36, "token": self.token}
)
good_reset_req = self.request_factory.get(url)
process_request(good_reset_req)
good_reset_req.user = AnonymousUser()
password_reset_confirm_wrapper(good_reset_req, self.uidb36, self.token)
PasswordResetConfirmWrapper.as_view()(good_reset_req, uidb36=self.uidb36, token=self.token)
self.user = User.objects.get(pk=self.user.pk)
self.assertTrue(self.user.is_active)
@@ -348,10 +358,11 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
)
request_params = {'new_password1': 'password1', 'new_password2': 'password2'}
confirm_request = self.request_factory.post(url, data=request_params)
process_request(confirm_request)
confirm_request.user = self.user
# Make a password reset request with mismatching passwords.
resp = password_reset_confirm_wrapper(confirm_request, self.uidb36, self.token)
resp = PasswordResetConfirmWrapper.as_view()(confirm_request, uidb36=self.uidb36, token=self.token)
# Verify the response status code is: 200 with password reset fail and also verify that
# the user is not marked as active.
@@ -373,7 +384,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
)
reset_req = self.request_factory.get(url)
reset_req.user = self.user
resp = password_reset_confirm_wrapper(reset_req, self.uidb36, self.token)
resp = PasswordResetConfirmWrapper.as_view()(reset_req, uidb36=self.uidb36, token=self.token)
# Verify the response status code is: 200 with password reset fail and also verify that
# the user is not marked as active.
@@ -388,7 +399,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
)
for request in [self.request_factory.get(url), self.request_factory.post(url)]:
request.user = self.user
response = password_reset_confirm_wrapper(request, self.uidb36, self.token)
response = PasswordResetConfirmWrapper.as_view()(request, uidb36=self.uidb36, token=self.token)
assert response.context_data['err_msg'] == SYSTEM_MAINTENANCE_MSG
self.user.refresh_from_db()
assert not self.user.is_active
@@ -408,8 +419,10 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
password = u'p\u212bssword'
request_params = {'new_password1': password, 'new_password2': password}
confirm_request = self.request_factory.post(url, data=request_params)
process_request(confirm_request)
confirm_request.session[INTERNAL_RESET_SESSION_TOKEN] = self.token
confirm_request.user = self.user
__ = password_reset_confirm_wrapper(confirm_request, self.uidb36, self.token)
__ = PasswordResetConfirmWrapper.as_view()(confirm_request, uidb36=self.uidb36, token=self.token)
user = User.objects.get(pk=self.user.pk)
salt_val = user.password.split('$')[1]
@@ -445,11 +458,11 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
confirm_request.user = self.user
# Make a password reset request with minimum/maximum passwords characters.
response = password_reset_confirm_wrapper(confirm_request, self.uidb36, self.token)
response = PasswordResetConfirmWrapper.as_view()(confirm_request, uidb36=self.uidb36, token=self.token)
self.assertEqual(response.context_data['err_msg'], password_dict['error_message'])
@patch('openedx.core.djangoapps.user_authn.views.password_reset.password_reset_confirm')
@patch.object(PasswordResetConfirmView, 'dispatch')
@patch("openedx.core.djangoapps.site_configuration.helpers.get_value", fake_get_value)
def test_reset_password_good_token_configuration_override(self, reset_confirm):
"""
@@ -460,8 +473,9 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
kwargs={"uidb36": self.uidb36, "token": self.token}
)
good_reset_req = self.request_factory.get(url)
process_request(good_reset_req)
good_reset_req.user = self.user
password_reset_confirm_wrapper(good_reset_req, self.uidb36, self.token)
PasswordResetConfirmWrapper.as_view()(good_reset_req, uidb36=self.uidb36, token=self.token)
confirm_kwargs = reset_confirm.call_args[1]
self.assertEqual(confirm_kwargs['extra_context']['platform_name'], 'Fake University')
self.user = User.objects.get(pk=self.user.pk)
@@ -497,7 +511,8 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
reset_request = self.request_factory.get(reset_url)
reset_request.user = UserFactory.create()
self.assertRaises(Http404, password_reset_confirm_wrapper, reset_request, self.uidb36, self.token)
self.assertRaises(Http404, PasswordResetConfirmWrapper.as_view(), reset_request, uidb36=self.uidb36,
token=self.token)
@ddt.ddt