Move request_password_change to user_authn.

This commit is contained in:
Diana Huang
2019-11-20 16:39:06 -05:00
parent 6a443643c2
commit d472cd8bfe
8 changed files with 212 additions and 170 deletions

View File

@@ -8,8 +8,6 @@ from importlib import import_module
from django import forms
from django.conf import settings
from django.contrib.auth.forms import PasswordResetForm
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.core.exceptions import ValidationError
@@ -28,9 +26,8 @@ from openedx.core.djangoapps.theming.helpers import get_current_site
from openedx.core.djangoapps.user_api import accounts as accounts_settings
from openedx.core.djangoapps.user_api.accounts.utils import is_secondary_email_feature_enabled
from openedx.core.djangoapps.user_api.preferences.api import get_user_preference
from openedx.core.djangoapps.user_authn.views.password_reset import send_password_reset_email_for_user
from student.message_types import AccountRecovery as AccountRecoveryMessage
from student.models import AccountRecovery, CourseEnrollmentAllowed, email_exists_or_retired
from student.models import CourseEnrollmentAllowed, email_exists_or_retired
from util.password_policy_validators import validate_password
@@ -67,56 +64,6 @@ def send_account_recovery_email_for_user(user, request, email=None):
ace.send(msg)
class PasswordResetFormNoActive(PasswordResetForm):
error_messages = {
'unknown': _("That e-mail address doesn't have an associated "
"user account. Are you sure you've registered?"),
'unusable': _("The user account associated with this e-mail "
"address cannot reset the password."),
}
is_account_recovery = True
def clean_email(self):
"""
This is a literal copy from Django 1.4.5's django.contrib.auth.forms.PasswordResetForm
Except removing the requirement of active users
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
self.users_cache = User.objects.filter(email__iexact=email)
if len(self.users_cache) == 0 and is_secondary_email_feature_enabled():
# Check if user has entered the secondary email.
self.users_cache = User.objects.filter(
id__in=AccountRecovery.objects.filter(secondary_email__iexact=email, is_active=True).values_list('user')
)
self.is_account_recovery = not bool(self.users_cache)
if not len(self.users_cache):
raise forms.ValidationError(self.error_messages['unknown'])
if any((user.password.startswith(UNUSABLE_PASSWORD_PREFIX))
for user in self.users_cache):
raise forms.ValidationError(self.error_messages['unusable'])
return email
def save(self, # pylint: disable=arguments-differ
use_https=False,
token_generator=default_token_generator,
request=None,
**_kwargs):
"""
Generates a one-use only link for resetting password and sends to the
user.
"""
for user in self.users_cache:
if self.is_account_recovery:
send_password_reset_email_for_user(user, request)
else:
send_account_recovery_email_for_user(user, request, user.account_recovery.secondary_email)
class TrueCheckbox(widgets.CheckboxInput):
"""
A checkbox widget that only accepts "true" (case-insensitive) as true.

View File

@@ -65,7 +65,7 @@ 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_authn.message_types import PasswordReset
from openedx.core.djangolib.markup import HTML, Text
from student.forms import AccountCreationForm, PasswordResetFormNoActive
from student.forms import AccountCreationForm
from student.helpers import DISABLE_UNENROLL_CERT_STATES, cert_info, generate_activation_email_context
from student.message_types import EmailChange, EmailChangeConfirmation, RecoveryEmailCreate
from student.models import (
@@ -676,7 +676,7 @@ def password_change_request_handler(request):
if email:
try:
from openedx.core.djangoapps.user_api.accounts.api import request_password_change
from openedx.core.djangoapps.user_authn.views.password_reset import request_password_change
request_password_change(email, request.is_secure())
user = user if user.is_authenticated else get_user_from_email(email=email)
destroy_oauth_tokens(user)
@@ -746,6 +746,7 @@ def password_reset(request):
AUDIT_LOG.warning("Rate limit exceeded in password_reset")
return HttpResponseForbidden()
from openedx.core.djangoapps.user_authn.views.password_reset import PasswordResetFormNoActive
form = PasswordResetFormNoActive(request.POST)
if form.is_valid():
form.save(use_https=request.is_secure(),

View File

@@ -20,7 +20,7 @@ from six import text_type # pylint: disable=ungrouped-imports
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
from openedx.core.djangoapps.theming.helpers import get_current_request
from openedx.core.djangoapps.user_api import accounts, errors, forms, helpers
from openedx.core.djangoapps.user_api import accounts, errors, helpers
from openedx.core.djangoapps.user_api.config.waffle import PREVENT_AUTH_USER_WRITES, SYSTEM_MAINTENANCE_MSG, waffle
from openedx.core.djangoapps.user_api.errors import (
AccountUpdateError,
@@ -360,44 +360,6 @@ def activate_account(activation_key):
registration.activate()
@helpers.intercept_errors(errors.UserAPIInternalError, ignore_errors=[errors.UserAPIRequestError])
def request_password_change(email, is_secure):
"""Email a single-use link for performing a password reset.
Users must confirm the password change before we update their information.
Args:
email (str): An email address
orig_host (str): An originating host, extracted from a request with get_host
is_secure (bool): Whether the request was made with HTTPS
Returns:
None
Raises:
errors.UserNotFound
AccountRequestError
errors.UserAPIInternalError: the operation failed due to an unexpected error.
"""
# Binding data to a form requires that the data be passed as a dictionary
# to the Form class constructor.
form = forms.PasswordResetFormNoActive({'email': email})
# Validate that a user exists with the given email address.
if form.is_valid():
# Generate a single-use link for performing a password reset
# and email it to the user.
form.save(
from_email=configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL),
use_https=is_secure,
request=get_current_request(),
)
else:
# No user with the provided email address exists.
raise errors.UserNotFound
def get_name_validation_error(name):
"""Get the built-in validation error message for when
the user's real name is invalid in some way (we wonder how).

View File

@@ -30,7 +30,6 @@ from openedx.core.djangoapps.user_api.accounts import PRIVATE_VISIBILITY, USERNA
from openedx.core.djangoapps.user_api.accounts.api import (
activate_account,
get_account_settings,
request_password_change,
update_account_settings
)
from openedx.core.djangoapps.user_api.accounts.tests.retirement_helpers import ( # pylint: disable=unused-import
@@ -580,57 +579,6 @@ class AccountActivationAndPasswordChangeTest(CreateAccountMixin, TestCase):
with waffle().override(PREVENT_AUTH_USER_WRITES, True):
activate_account(activation_key)
@skip_unless_lms
def test_request_password_change(self):
# Create and activate an account
self.create_account(self.USERNAME, self.PASSWORD, self.EMAIL)
self.assertEqual(len(mail.outbox), 1)
user = User.objects.get(username=self.USERNAME)
activation_key = self.get_activation_key(user)
activate_account(activation_key)
request = RequestFactory().post('/password')
request.user = Mock()
request.site = SiteFactory()
with patch('crum.get_current_request', return_value=request):
# Request a password change
request_password_change(self.EMAIL, self.IS_SECURE)
# Verify that a new email message has been sent
self.assertEqual(len(mail.outbox), 2)
# Verify that the body of the message contains something that looks
# like an activation link
email_body = mail.outbox[0].body
result = re.search(r'(?P<url>https?://[^\s]+)', email_body)
self.assertIsNot(result, None)
@skip_unless_lms
def test_request_password_change_invalid_user(self):
with self.assertRaises(UserNotFound):
request_password_change(self.EMAIL, self.IS_SECURE)
# Verify that no email messages have been sent
self.assertEqual(len(mail.outbox), 0)
@skip_unless_lms
def test_request_password_change_inactive_user(self):
# Create an account, but do not activate it
self.create_account(self.USERNAME, self.PASSWORD, self.EMAIL)
self.assertEqual(len(mail.outbox), 1)
request = RequestFactory().post('/password')
request.user = Mock()
request.site = SiteFactory()
with patch('crum.get_current_request', return_value=request):
request_password_change(self.EMAIL, self.IS_SECURE)
# Verify that the activation email was still sent
self.assertEqual(len(mail.outbox), 2)
def _assert_is_datetime(self, timestamp):
"""
Internal helper to validate the type of the provided timestamp

View File

@@ -1,8 +0,0 @@
# pylint: disable=unused-import
# TODO: eventually move this implementation into the user_api
"""
Django Administration forms module
"""
from __future__ import absolute_import
from student.forms import PasswordResetFormNoActive

View File

@@ -1,6 +1,12 @@
""" Handles password resets logic. """
from django import forms
from django.contrib.auth.forms import PasswordResetForm
from django.conf import settings
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.http import HttpResponse
from django.utils.decorators import method_decorator
@@ -16,12 +22,16 @@ from rest_framework.views import APIView
from openedx.core.djangoapps.ace_common.template_context import get_base_template_context
from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
from openedx.core.djangoapps.theming.helpers import get_current_site
from openedx.core.djangoapps.user_api import accounts
from openedx.core.djangoapps.theming.helpers import get_current_request, get_current_site
from openedx.core.djangoapps.user_api import accounts, errors, helpers
from openedx.core.djangoapps.user_api.accounts.utils import is_secondary_email_feature_enabled
from openedx.core.djangoapps.user_api.helpers import FormDescription
from openedx.core.djangoapps.user_api.preferences.api import get_user_preference
from openedx.core.djangoapps.user_authn.message_types import PasswordReset
from student.models import AccountRecovery
from student.forms import send_account_recovery_email_for_user
def get_password_reset_form():
"""Return a description of the password reset form.
@@ -69,18 +79,6 @@ def get_password_reset_form():
return form_desc
class PasswordResetView(APIView):
"""HTTP end-point for GETting a description of the password reset form. """
# This end-point is available to anonymous users,
# so do not require authentication.
authentication_classes = []
@method_decorator(ensure_csrf_cookie)
def get(self, request):
return HttpResponse(get_password_reset_form().to_json(), content_type="application/json")
def send_password_reset_email_for_user(user, request, preferred_email=None):
"""
Send out a password reset email for the given user.
@@ -112,3 +110,108 @@ def send_password_reset_email_for_user(user, request, preferred_email=None):
user_context=message_context,
)
ace.send(msg)
class PasswordResetFormNoActive(PasswordResetForm):
"""
A modified version of the default Django password reset form to handle
unknown or unusable email addresses without leaking data.
"""
error_messages = {
'unknown': _("That e-mail address doesn't have an associated "
"user account. Are you sure you've registered?"),
'unusable': _("The user account associated with this e-mail "
"address cannot reset the password."),
}
is_account_recovery = True
users_cache = []
def clean_email(self):
"""
This is a literal copy from Django 1.4.5's django.contrib.auth.forms.PasswordResetForm
Except removing the requirement of active users
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
self.users_cache = User.objects.filter(email__iexact=email)
if not self.users_cache and is_secondary_email_feature_enabled():
# Check if user has entered the secondary email.
self.users_cache = User.objects.filter(
id__in=AccountRecovery.objects.filter(secondary_email__iexact=email, is_active=True).values_list('user')
)
self.is_account_recovery = not bool(self.users_cache)
if not self.users_cache:
raise forms.ValidationError(self.error_messages['unknown'])
if any((user.password.startswith(UNUSABLE_PASSWORD_PREFIX))
for user in self.users_cache):
raise forms.ValidationError(self.error_messages['unusable'])
return email
def save(self, # pylint: disable=arguments-differ
use_https=False,
token_generator=default_token_generator,
request=None,
**_kwargs):
"""
Generates a one-use only link for resetting password and sends to the
user.
"""
for user in self.users_cache:
if self.is_account_recovery:
send_password_reset_email_for_user(user, request)
else:
send_account_recovery_email_for_user(user, request, user.account_recovery.secondary_email)
class PasswordResetView(APIView):
"""HTTP end-point for GETting a description of the password reset form. """
# This end-point is available to anonymous users,
# so do not require authentication.
authentication_classes = []
@method_decorator(ensure_csrf_cookie)
def get(self, request):
return HttpResponse(get_password_reset_form().to_json(), content_type="application/json")
@helpers.intercept_errors(errors.UserAPIInternalError, ignore_errors=[errors.UserAPIRequestError])
def request_password_change(email, is_secure):
"""Email a single-use link for performing a password reset.
Users must confirm the password change before we update their information.
Args:
email (str): An email address
orig_host (str): An originating host, extracted from a request with get_host
is_secure (bool): Whether the request was made with HTTPS
Returns:
None
Raises:
errors.UserNotFound
AccountRequestError
errors.UserAPIInternalError: the operation failed due to an unexpected error.
"""
# Binding data to a form requires that the data be passed as a dictionary
# to the Form class constructor.
form = PasswordResetFormNoActive({'email': email})
# Validate that a user exists with the given email address.
if form.is_valid():
# Generate a single-use link for performing a password reset
# and email it to the user.
form.save(
from_email=configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL),
use_https=is_secure,
request=get_current_request(),
)
else:
# No user with the provided email address exists.
raise errors.UserNotFound

View File

@@ -0,0 +1,89 @@
# -*- coding: utf-8 -*-
"""
Tests for user authorization password-related functionality.
"""
import re
from mock import Mock, patch
from django.core import mail
from django.contrib.auth.models import User
from django.test import TestCase
from django.test.client import RequestFactory
from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory
from openedx.core.djangoapps.user_api.accounts.tests.test_api import CreateAccountMixin
from openedx.core.djangoapps.user_api.accounts.api import (
activate_account,
)
from openedx.core.djangoapps.user_api.errors import UserNotFound
from openedx.core.djangoapps.user_authn.views.password_reset import request_password_change
from openedx.core.djangolib.testing.utils import skip_unless_lms
from student.models import Registration
class TestRequestPasswordChange(CreateAccountMixin, TestCase):
"""
Tests for users who request a password change.
"""
USERNAME = u'claire-underwood'
PASSWORD = u'ṕáśśẃőŕd'
EMAIL = u'claire+underwood@example.com'
IS_SECURE = False
def get_activation_key(self, user):
registration = Registration.objects.get(user=user)
return registration.activation_key
@skip_unless_lms
def test_request_password_change(self):
# Create and activate an account
self.create_account(self.USERNAME, self.PASSWORD, self.EMAIL)
self.assertEqual(len(mail.outbox), 1)
user = User.objects.get(username=self.USERNAME)
activation_key = self.get_activation_key(user)
activate_account(activation_key)
request = RequestFactory().post('/password')
request.user = Mock()
request.site = SiteFactory()
with patch('crum.get_current_request', return_value=request):
# Request a password change
request_password_change(self.EMAIL, self.IS_SECURE)
# Verify that a new email message has been sent
self.assertEqual(len(mail.outbox), 2)
# Verify that the body of the message contains something that looks
# like an activation link
email_body = mail.outbox[0].body
result = re.search(r'(?P<url>https?://[^\s]+)', email_body)
self.assertIsNot(result, None)
@skip_unless_lms
def test_request_password_change_invalid_user(self):
with self.assertRaises(UserNotFound):
request_password_change(self.EMAIL, self.IS_SECURE)
# Verify that no email messages have been sent
self.assertEqual(len(mail.outbox), 0)
@skip_unless_lms
def test_request_password_change_inactive_user(self):
# Create an account, but do not activate it
self.create_account(self.USERNAME, self.PASSWORD, self.EMAIL)
self.assertEqual(len(mail.outbox), 1)
request = RequestFactory().post('/password')
request.user = Mock()
request.site = SiteFactory()
with patch('crum.get_current_request', return_value=request):
request_password_change(self.EMAIL, self.IS_SECURE)
# Verify that the activation email was still sent
self.assertEqual(len(mail.outbox), 2)

View File

@@ -167,7 +167,7 @@ class UserAccountUpdateTest(CacheIsolationTestCase, UrlResetMixin):
assert response_dict['success']
def test_password_change_failure(self):
with mock.patch('openedx.core.djangoapps.user_api.accounts.api.request_password_change',
with mock.patch('openedx.core.djangoapps.user_authn.views.password_reset.request_password_change',
side_effect=UserAPIInternalError):
self._change_password()
self.assertRaises(UserAPIInternalError)