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

Consolidate recovery assistance forms

Browse Source
This commit is contained in:
Saleem Latif
2019-01-16 16:26:13 +05:00
parent 33fab4b1e4
commit 46d97caa47
18 changed files with 69 additions and 709 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
openedx/core/djangoapps/user_api/accounts/api.py
View File

@@ -486,36 +486,6 @@ def request_password_change(email, is_secure):
raise errors.UserNotFound
@helpers.intercept_errors(errors.UserAPIInternalError, ignore_errors=[errors.UserAPIRequestError])
def request_account_recovery(email, is_secure):
"""
Email a single-use link for performing a password reset so users can login with new email and password.
Arguments:
email (str): An email address
is_secure (bool): Whether the request was made with HTTPS
Raises:
errors.UserNotFound: Raised if secondary email address does not exist.
"""
# Binding data to a form requires that the data be passed as a dictionary
# to the Form class constructor.
form = student_forms.AccountRecoveryForm({'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).

48
openedx/core/djangoapps/user_api/api.py
View File

@@ -66,54 +66,6 @@ def get_password_reset_form():
return form_desc
def get_account_recovery_form():
"""
Return a description of the password reset, using secondary email, form.
This decouples clients from the API definition:
if the API decides to modify the form, clients won't need
to be updated.
See `user_api.helpers.FormDescription` for examples
of the JSON-encoded form description.
Returns:
HttpResponse
"""
form_desc = FormDescription("post", reverse("account_recovery"))
# Translators: This label appears above a field on the password reset
# form meant to hold the user's email address.
email_label = _(u"Secondary email")
# Translators: This example email address is used as a placeholder in
# a field on the password reset form meant to hold the user's email address.
email_placeholder = _(u"username@domain.com")
# Translators: These instructions appear on the password reset form,
# immediately below a field meant to hold the user's email address.
email_instructions = _(
u"Secondary email address you registered with {platform_name} using account settings page"
).format(
platform_name=configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME)
)
form_desc.add_field(
"email",
field_type="email",
label=email_label,
placeholder=email_placeholder,
instructions=email_instructions,
restrictions={
"min_length": accounts.EMAIL_MIN_LENGTH,
"max_length": accounts.EMAIL_MAX_LENGTH,
}
)
return form_desc
def get_login_session_form(request):
"""Return a description of the login form.

3
openedx/core/djangoapps/user_authn/views/login_form.py
View File

@@ -22,7 +22,6 @@ from openedx.core.djangoapps.theming.helpers import is_request_in_themed_site
from openedx.core.djangoapps.user_api.accounts.utils import is_secondary_email_feature_enabled
from openedx.core.djangoapps.user_api.api import (
RegistrationFormFactory,
get_account_recovery_form,
get_login_session_form,
get_password_reset_form,
)
@@ -138,7 +137,6 @@ def login_and_registration_form(request, initial_mode="login"):
'login_form_desc': json.loads(form_descriptions['login']),
'registration_form_desc': json.loads(form_descriptions['registration']),
'password_reset_form_desc': json.loads(form_descriptions['password_reset']),
'account_recovery_form_desc': json.loads(form_descriptions['account_recovery']),
'account_creation_allowed': configuration_helpers.get_value(
'ALLOW_PUBLIC_ACCOUNT_CREATION', settings.FEATURES.get('ALLOW_PUBLIC_ACCOUNT_CREATION', True)),
'is_account_recovery_feature_enabled': is_secondary_email_feature_enabled()
@@ -177,7 +175,6 @@ def _get_form_descriptions(request):
return {
'password_reset': get_password_reset_form().to_json(),
'account_recovery': get_account_recovery_form().to_json(),
'login': get_login_session_form(request).to_json(),
'registration': RegistrationFormFactory().get_registration_form(request).to_json()
}

88
openedx/core/djangoapps/user_authn/views/tests/test_views.py
View File

@@ -17,7 +17,6 @@ from django.contrib.messages.middleware import MessageMiddleware
from django.contrib.sessions.middleware import SessionMiddleware
from django.core import mail
from django.core.files.uploadedfile import SimpleUploadedFile
from django.http import Http404
from django.urls import reverse
from django.test import TestCase
from django.test.client import RequestFactory
@@ -30,7 +29,6 @@ from provider.oauth2.models import AccessToken as dop_access_token
from provider.oauth2.models import RefreshToken as dop_refresh_token
from testfixtures import LogCapture
from waffle.models import Switch
from waffle.testutils import override_switch
from course_modes.models import CourseMode
from openedx.core.djangoapps.user_authn.views.login_form import login_and_registration_form
@@ -261,83 +259,6 @@ class UserAccountUpdateTest(CacheIsolationTestCase, UrlResetMixin):
response = getattr(self.client, method)(url)
self.assertEqual(response.status_code, 405)
@override_switch(ENABLE_SECONDARY_EMAIL_FEATURE_SWITCH, active=False)
def test_404_if_account_recovery_not_enabled(self):
with mock.patch('openedx.core.djangoapps.user_api.accounts.api.request_account_recovery',
side_effect=UserAPIInternalError):
self._recover_account()
self.assertRaises(Http404)
def test_account_recovery_failure(self):
with mock.patch('openedx.core.djangoapps.user_api.accounts.api.request_account_recovery',
side_effect=UserAPIInternalError):
self._recover_account()
self.assertRaises(UserAPIInternalError)
@override_settings(FEATURES=FEATURES_WITH_FAILED_PASSWORD_RESET_EMAIL)
def test_account_recovery_failure_email(self):
"""Test that log message is added when email does not match any in the system."""
# Log the user out
self.client.logout()
with LogCapture(LOGGER_NAME, level=logging.INFO) as logger:
bad_email = 'doesnotexist@example.com'
response = self._recover_account(email=bad_email)
self.assertEqual(response.status_code, 200)
logger.check(
(
LOGGER_NAME,
"WARNING", "Account recovery attempt via invalid secondary email '{email}'.".format(
email=bad_email
)
)
)
@override_settings(FEATURES=FEATURES_WITH_FAILED_PASSWORD_RESET_EMAIL)
def test_account_recovery_failure_not_active(self):
"""Test that log message is added when email does not match any active account recovery records."""
# Log the user out
self.client.logout()
self.account_recovery.is_active = False
self.account_recovery.save()
with LogCapture(LOGGER_NAME, level=logging.INFO) as logger:
response = self._recover_account(email=self.account_recovery.secondary_email)
self.assertEqual(response.status_code, 200)
logger.check(
(
LOGGER_NAME,
"WARNING", "Account recovery attempt via invalid secondary email '{email}'.".format(
email=self.account_recovery.secondary_email
)
)
)
def test_password_change_rate_limited_during_account_recovery(self):
# Log out the user created during test setup, to prevent the view from
# selecting the logged-in user's email address over the email provided
# in the POST data
self.client.logout()
# Make many consecutive bad requests in an attempt to trigger the rate limiter
for __ in xrange(self.INVALID_ATTEMPTS):
self._recover_account(email=self.NEW_EMAIL)
response = self._recover_account(email=self.NEW_EMAIL)
self.assertEqual(response.status_code, 403)
@ddt.data(
('post', 'account_recovery', []),
)
@ddt.unpack
def test_require_http_method_during_account_recovery(self, correct_method, url_name, args):
wrong_methods = {'get', 'put', 'post', 'head', 'options', 'delete'} - {correct_method}
url = reverse(url_name, args=args)
for method in wrong_methods:
response = getattr(self.client, method)(url)
self.assertEqual(response.status_code, 405)
def _change_password(self, email=None):
"""Request to change the user's password. """
data = {}
@@ -347,15 +268,6 @@ class UserAccountUpdateTest(CacheIsolationTestCase, UrlResetMixin):
return self.client.post(path=reverse('password_change_request'), data=data)
def _recover_account(self, email=None):
"""Request to create the user's password. """
data = {}
if email:
data['email'] = email
return self.client.post(path=reverse('account_recovery'), data=data)
def _create_dop_tokens(self, user=None):
"""Create dop access token for given user if user provided else for default user."""
if not user: