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

Merge pull request #19500 from edx/saleem-latif/ENT-1118

Browse Source 
ENT-1118: Update sign in email address for continued access
This commit is contained in:
Saleem Latif
2018-12-31 12:27:46 +05:00
committed by GitHub
parent f44c68fb52 eaf93d5978
commit c2a218f379
16 changed files with 398 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

46
common/djangoapps/student/forms.py
View File

@@ -26,7 +26,7 @@ from openedx.core.djangoapps.site_configuration import helpers as configuration_
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.preferences.api import get_user_preference
from student.message_types import PasswordReset
from student.message_types import AccountRecovery as AccountRecoveryMessage, PasswordReset
from student.models import AccountRecovery, CourseEnrollmentAllowed, email_exists_or_retired
from util.password_policy_validators import validate_password
@@ -64,6 +64,38 @@ def send_password_reset_email_for_user(user, request, preferred_email=None):
ace.send(msg)
def send_account_recovery_email_for_user(user, request, email=None):
"""
Send out a account recovery email for the given user.
Arguments:
user (User): Django User object
request (HttpRequest): Django request object
email (str): Send email to this address.
"""
site = get_current_site()
message_context = get_base_template_context(site)
message_context.update({
'request': request, # Used by google_analytics_tracking_pixel
'platform_name': configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME),
'reset_link': '{protocol}://{site}{link}'.format(
protocol='https' if request.is_secure() else 'http',
site=configuration_helpers.get_value('SITE_NAME', settings.SITE_NAME),
link=reverse('account_recovery_confirm', kwargs={
'uidb36': int_to_base36(user.id),
'token': default_token_generator.make_token(user),
}),
)
})
msg = AccountRecoveryMessage().personalize(
recipient=Recipient(user.username, email),
language=get_user_preference(user, LANGUAGE_KEY),
user_context=message_context,
)
ace.send(msg)
class PasswordResetFormNoActive(PasswordResetForm):
error_messages = {
'unknown': _("That e-mail address doesn't have an associated "
@@ -138,6 +170,18 @@ class AccountRecoveryForm(PasswordResetFormNoActive):
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 setting the password and sends to the
user.
"""
for user in self.users_cache:
send_account_recovery_email_for_user(user, request, user.account_recovery.secondary_email)
class TrueCheckbox(widgets.CheckboxInput):
"""

7
common/djangoapps/student/message_types.py
View File

@@ -12,6 +12,13 @@ class PasswordReset(BaseMessageType):
self.options['transactional'] = True
class AccountRecovery(BaseMessageType):
def __init__(self, *args, **kwargs):
super(AccountRecovery, self).__init__(*args, **kwargs)
self.options['transactional'] = True
class EmailChange(BaseMessageType):
def __init__(self, *args, **kwargs):
super(EmailChange, self).__init__(*args, **kwargs)

10
common/djangoapps/student/tests/factories.py
View File

@@ -13,6 +13,7 @@ from pytz import UTC
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
from student.models import (
AccountRecovery,
CourseAccessRole,
CourseEnrollment,
CourseEnrollmentAllowed,
@@ -200,3 +201,12 @@ class PermissionFactory(DjangoModelFactory):
codename = factory.Faker('codename')
content_type = factory.SubFactory(ContentTypeFactory)
class AccountRecoveryFactory(DjangoModelFactory):
class Meta(object):
model = AccountRecovery
django_get_or_create = ('user',)
user = None
secondary_email = factory.Sequence(u'robot+test+recovery+{0}@edx.org'.format)

5
common/djangoapps/student/urls.py
View File

@@ -29,6 +29,11 @@ urlpatterns = [
views.password_reset_confirm_wrapper,
name='password_reset_confirm',
),
url(
r'^account_recovery_confirm/(?P<uidb36>[0-9A-Za-z]+)-(?P<token>.+)/$',
views.account_recovery_confirm_wrapper,
name='account_recovery_confirm',
),
url(r'^course_run/{}/refund_status$'.format(settings.COURSE_ID_PATTERN),
views.course_run_refund_status,

127
common/djangoapps/student/views/management.py
View File

@@ -765,6 +765,7 @@ def account_recovery_request_handler(request):
AUDIT_LOG.warning(
"Account recovery attempt via invalid secondary email '{email}'.".format(email=email)
)
limiter.tick_bad_request_counter(request)
return HttpResponse(status=200)
else:
@@ -941,6 +942,132 @@ def password_reset_confirm_wrapper(request, uidb36=None, token=None):
return response
def account_recovery_confirm_wrapper(request, uidb36=None, token=None):
"""
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.
"""
# 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 secondary email feature is enabled.
if not is_secondary_email_feature_enabled():
raise Http404
try:
uid_int = base36_to_int(uidb36)
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,
template_name='account_recovery/password_create_confirm.html'
)
if request.method == 'POST':
# 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
# field so it passes the equivalence check that new_password1 == new_password2
# In order to switch out of having to do this copy, we would want to move the normalize_password code into
# a custom User model's set_password method to ensure it is always happening upon calling set_password.
request.POST = request.POST.copy()
request.POST['new_password1'] = normalize_password(request.POST['new_password1'])
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 creation unsuccessful'),
'err_msg': ' '.join(err.messages),
}
context.update(platform_name)
return TemplateResponse(
request, 'account_recovery/password_create_confirm.html', context
)
# remember what the old password hash is before we call down
old_password_hash = user.password
response = password_reset_confirm(
request,
uidb64=uidb64,
token=token,
extra_context=platform_name,
template_name='account_recovery/password_create_confirm.html',
post_reset_redirect='signin_user',
)
# 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 create password for user [%s] because form is not valid. '
u'A possible cause is that the user had an invalid create token',
user.username,
)
response.context_data['err_msg'] = _('Error in creating your password. Please try again.')
return response
# get the updated user
updated_user = User.objects.get(id=uid_int)
updated_user.email = updated_user.account_recovery.secondary_email
updated_user.save()
if response.status_code == 302:
messages.success(
request,
HTML(_(
'{html_start}Password Creation Complete{html_end}'
'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,
template_name='account_recovery/password_create_confirm.html',
)
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
def validate_new_email(user, new_email):
"""
Given a new email for a user, does some basic verification of the new address If any issues are encountered

28
common/templates/student/edx_ace/accountrecovery/email/body.html Normal file
View File

@@ -0,0 +1,28 @@
{% extends 'ace_common/edx_ace/common/base_body.html' %}
{% load i18n %}
{% load static %}
{% block content %}
<table width="100%" align="left" border="0" cellpadding="0" cellspacing="0" role="presentation">
<tr>
<td>
<h1>
{% trans "Create Password" %}
</h1>
<p style="color: rgba(0,0,0,.75);">
{% blocktrans %}You're receiving this e-mail because you requested to create a password for your user account at {{ platform_name }}.{% endblocktrans %}
<br />
</p>
<p style="color: rgba(0,0,0,.75);">
{% trans "If you didn't request this change, you can disregard this email - we have not yet created your password." %}
<br />
</p>
{% trans "Create my Password" as course_cta_text %}
{% include "ace_common/edx_ace/common/return_to_course_cta.html" with course_cta_text=course_cta_text course_cta_url=reset_link %}
</td>
</tr>
</table>
{% endblock %}

12
common/templates/student/edx_ace/accountrecovery/email/body.txt Normal file
View File

@@ -0,0 +1,12 @@
{% load i18n %}{% autoescape off %}
{% blocktrans %}You're receiving this e-mail because you requested to create a password for your user account at {{ platform_name }}.{% endblocktrans %}
{% trans "Please go to the following page and choose a new password:" %}
{{ reset_link }}
{% trans "If you didn't request this change, you can disregard this email - we have not yet created your password." %}
{% trans "Thanks for using our site!" %}
{% blocktrans %}The {{ platform_name }} Team{% endblocktrans %}
{% endautoescape %}

1
common/templates/student/edx_ace/accountrecovery/email/from_name.txt Normal file
View File

@@ -0,0 +1 @@
{{ platform_name }}

1
common/templates/student/edx_ace/accountrecovery/email/head.html Normal file
View File

@@ -0,0 +1 @@
{% extends 'ace_common/edx_ace/common/base_head.html' %}

4
common/templates/student/edx_ace/accountrecovery/email/subject.txt Normal file
View File

@@ -0,0 +1,4 @@
{% load i18n %}
{% autoescape off %}
{% blocktrans trimmed %}Create password on {{ platform_name }}{% endblocktrans %}
{% endautoescape %}

8
lms/static/js/student_account/components/PasswordResetConfirmation.jsx
View File

@@ -85,7 +85,7 @@ class PasswordResetConfirmation extends React.Component {
<form id="passwordreset-form" method="post" action="">
<h2 className="section-title lines">
<span className="text">
{gettext('Reset Your Password')}
{this.props.formTitle}
</span>
</h2>
@@ -121,7 +121,7 @@ class PasswordResetConfirmation extends React.Component {
<Button
type="submit"
className={['action', 'action-primary', 'action-update', 'js-reset']}
label={gettext('Reset My Password')}
label={this.props.primaryActionButtonLabel}
/>
</form>
</div>
@@ -133,10 +133,14 @@ class PasswordResetConfirmation extends React.Component {
PasswordResetConfirmation.propTypes = {
csrfToken: PropTypes.string.isRequired,
errorMessage: PropTypes.string,
primaryActionButtonLabel: PropTypes.string,
formTitle: PropTypes.string,
};
PasswordResetConfirmation.defaultProps = {
errorMessage: '',
primaryActionButtonLabel: gettext('Reset My Password'),
formTitle: gettext('Reset Your Password'),
};
export { PasswordResetConfirmation }; // eslint-disable-line import/prefer-default-export

5
lms/static/js/student_account/views/AccessView.js
View File

@@ -59,6 +59,7 @@
// Account activation messages
this.accountActivationMessages = options.account_activation_messages || [];
this.accountRecoveryMessages = options.account_recovery_messages || [];
if (options.login_redirect_url) {
this.nextUrl = options.login_redirect_url;
@@ -99,9 +100,10 @@
// there is no need to show it again, if the user changes mode:
this.thirdPartyAuth.errorMessage = null;
// Once the account activation messages have been shown once,
// Once the account activation/account recovery messages have been shown once,
// there is no need to show it again, if the user changes mode:
this.accountActivationMessages = [];
this.accountRecoveryMessages = [];
},
render: function() {
@@ -148,6 +150,7 @@
accountRecoveryModel: this.accountRecoveryModel,
thirdPartyAuth: this.thirdPartyAuth,
accountActivationMessages: this.accountActivationMessages,
accountRecoveryMessages: this.accountRecoveryMessages,
platformName: this.platformName,
supportURL: this.supportURL,
passwordResetSupportUrl: this.passwordResetSupportUrl,

10
lms/static/js/student_account/views/LoginView.js
View File

@@ -51,6 +51,7 @@
this.passwordResetSupportUrl = data.passwordResetSupportUrl;
this.createAccountOption = data.createAccountOption;
this.accountActivationMessages = data.accountActivationMessages;
this.accountRecoveryMessages = data.accountRecoveryMessages;
this.hideAuthWarnings = data.hideAuthWarnings;
this.pipelineUserDetails = data.pipelineUserDetails;
this.enterpriseName = data.enterpriseName;
@@ -111,13 +112,18 @@
// Display account activation success or error messages.
this.renderAccountActivationMessages();
this.renderAccountRecoveryMessages();
},
renderAccountActivationMessages: function() {
_.each(this.accountActivationMessages, this.renderAccountActivationMessage, this);
_.each(this.accountActivationMessages, this.renderMessage, this);
},
renderAccountActivationMessage: function(message) {
renderAccountRecoveryMessages: function() {
_.each(this.accountRecoveryMessages, this.renderMessage, this);
},
renderMessage: function(message) {
this.renderFormFeedback(this.formStatusTpl, {
jsHook: message.tags,
message: HtmlUtils.HTML(message.message)

56
lms/templates/account_recovery/password_create_confirm.html Normal file
View File

@@ -0,0 +1,56 @@
## mako
<%page expression_filter="h"/>
<%!
from django.utils.translation import ugettext as _
from openedx.core.djangolib.js_utils import js_escaped_string
from openedx.core.djangolib.markup import HTML, Text
%>
<%inherit file="../main.html"/>
<%namespace name='static' file='../static_content.html'/>
<%block name="title">
<title>${_("Create Your {platform_name} Password").format(platform_name=platform_name)}</title>
</%block>
<%block name="head_extra">
<link type="text/css" rel="stylesheet" href="${STATIC_URL}paragon/static/paragon.min.css">
</%block>
<%block name="bodyclass">view-passwordreset</%block>
<%block name="body">
<div id="password-reset-confirm-container" class="login-register-content login-register">
% if validlink:
${static.renderReact(
component="PasswordResetConfirmation",
id="password-reset-confirm-react",
props={
'csrfToken': csrf_token,
'errorMessage': js_escaped_string(err_msg) if err_msg else '',
'primaryActionButtonLabel': 'Create My Password',
'formTitle': 'Create Your Password',
},
)}
% else:
<div class="status submission-error">
<h4 class="message-title">${_("Invalid Password Create Link")}</h4>
<ul class="message-copy">
${Text(_((
"This password create link is invalid. It may have been used already. "
"To create your password, go to the {start_link}sign-in{end_link} page and "
"select {start_strong}Recovery your account{end_strong}."
))).format(
start_link=HTML('<a href="/login">'),
end_link=HTML('</a>'),
start_strong=HTML('<strong>'),
end_strong=HTML('</strong>')
)
}
</ul>
</div>
% endif
</div>
</%block>

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

@@ -110,6 +110,12 @@ def login_and_registration_form(request, initial_mode="login"):
} for message in messages.get_messages(request) if 'account-activation' in message.tags
]
account_recovery_messages = [
{
'message': message.message, 'tags': message.tags
} for message in messages.get_messages(request) if 'account-recovery' in message.tags
]
# Otherwise, render the combined login/registration page
context = {
'data': {
@@ -123,6 +129,7 @@ def login_and_registration_form(request, initial_mode="login"):
'PASSWORD_RESET_SUPPORT_LINK', settings.PASSWORD_RESET_SUPPORT_LINK
) or settings.SUPPORT_SITE_LINK,
'account_activation_messages': account_activation_messages,
'account_recovery_messages': account_recovery_messages,
# Include form descriptions retrieved from the user API.
# We could have the JS client make these requests directly,

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

@@ -17,6 +17,7 @@ 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
@@ -28,6 +29,8 @@ from oauth2_provider.models import RefreshToken as dot_refresh_token
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
@@ -36,9 +39,11 @@ from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin
from openedx.core.djangoapps.theming.tests.test_util import with_comprehensive_theme_context
from openedx.core.djangoapps.user_api.accounts.api import activate_account, create_account
from openedx.core.djangoapps.user_api.errors import UserAPIInternalError
from openedx.core.djangoapps.user_api.accounts.utils import ENABLE_SECONDARY_EMAIL_FEATURE_SWITCH
from openedx.core.djangolib.js_utils import dump_js_escaped_json
from openedx.core.djangolib.markup import HTML, Text
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms
from student.tests.factories import AccountRecoveryFactory
from third_party_auth.tests.testutil import ThirdPartyAuthTestMixin, simulate_running_pipeline
from util.testing import UrlResetMixin
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
@@ -76,6 +81,12 @@ class UserAccountUpdateTest(CacheIsolationTestCase, UrlResetMixin):
activation_key = create_account(self.USERNAME, self.OLD_PASSWORD, self.OLD_EMAIL)
activate_account(activation_key)
self.account_recovery = AccountRecoveryFactory.create(user=User.objects.get(email=self.OLD_EMAIL))
self.enable_account_recovery_switch = Switch.objects.create(
name=ENABLE_SECONDARY_EMAIL_FEATURE_SWITCH,
active=True
)
# Login
result = self.client.login(username=self.USERNAME, password=self.OLD_PASSWORD)
self.assertTrue(result)
@@ -250,6 +261,63 @@ 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 a password reset failure email notification is sent, when enabled."""
# 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
)
)
)
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 = {}
@@ -259,6 +327,15 @@ 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: