diff --git a/common/djangoapps/student/migrations/0019_auto_20181221_0540.py b/common/djangoapps/student/migrations/0019_auto_20181221_0540.py
new file mode 100644
index 0000000000000000000000000000000000000000..2b351a384d7ce41b4f10f3c2ee6d9a104cebd097
--- /dev/null
+++ b/common/djangoapps/student/migrations/0019_auto_20181221_0540.py
@@ -0,0 +1,34 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.16 on 2018-12-21 10:40
+from __future__ import unicode_literals
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+import openedx.core.djangolib.model_mixins
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+ ('student', '0018_remove_password_history'),
+ ]
+
+ operations = [
+ migrations.CreateModel(
+ name='PendingSecondaryEmailChange',
+ fields=[
+ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+ ('new_secondary_email', models.CharField(blank=True, db_index=True, max_length=255)),
+ ('activation_key', models.CharField(db_index=True, max_length=32, unique=True, verbose_name=b'activation key')),
+ ('user', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)),
+ ],
+ bases=(openedx.core.djangolib.model_mixins.DeletableByUserValue, models.Model),
+ ),
+ migrations.AddField(
+ model_name='accountrecovery',
+ name='is_active',
+ field=models.BooleanField(default=False),
+ ),
+ ]
diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py
index 2203aba9db6c8547332dd72a2ebca30268dc8f64..9c5387a2c80c542330d7f7afe2683cee673318aa 100644
--- a/common/djangoapps/student/models.py
+++ b/common/djangoapps/student/models.py
@@ -785,6 +785,15 @@ class PendingEmailChange(DeletableByUserValue, models.Model):
return self.activation_key
+class PendingSecondaryEmailChange(DeletableByUserValue, models.Model):
+ """
+ This model keeps track of pending requested changes to a user's secondary email address.
+ """
+ user = models.OneToOneField(User, unique=True, db_index=True, on_delete=models.CASCADE)
+ new_secondary_email = models.CharField(blank=True, max_length=255, db_index=True)
+ activation_key = models.CharField(('activation key'), max_length=32, unique=True, db_index=True)
+
+
EVENT_NAME_ENROLLMENT_ACTIVATED = 'edx.course.enrollment.activated'
EVENT_NAME_ENROLLMENT_DEACTIVATED = 'edx.course.enrollment.deactivated'
EVENT_NAME_ENROLLMENT_MODE_CHANGED = 'edx.course.enrollment.mode_changed'
@@ -2692,6 +2701,7 @@ class AccountRecovery(models.Model):
null=False,
blank=False,
)
+ is_active = models.BooleanField(default=False)
class Meta(object):
db_table = "auth_accountrecovery"
diff --git a/common/djangoapps/student/tests/test_email.py b/common/djangoapps/student/tests/test_email.py
index f0ba4135b4dfbaea7740abb43b611af90ca77c0f..7dd29304f99641f8c290e2145bbcab004bfd5d3f 100644
--- a/common/djangoapps/student/tests/test_email.py
+++ b/common/djangoapps/student/tests/test_email.py
@@ -575,3 +575,112 @@ class EmailChangeConfirmationTests(EmailTestMixin, CacheIsolationMixin, Transact
confirm_email_change(self.request, self.key)
mock_rollback.assert_called_with()
+
+
+@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', "Test only valid in LMS")
+class SecondaryEmailChangeRequestTests(EventTestMixin, EmailTemplateTagMixin, CacheIsolationTestCase):
+ """
+ Test changing a user's email address
+ """
+
+ def setUp(self, tracker='student.views.management.tracker'):
+ super(SecondaryEmailChangeRequestTests, self).setUp(tracker)
+ self.user = UserFactory.create()
+ self.new_secondary_email = 'new.secondary.email@edx.org'
+ self.req_factory = RequestFactory()
+ self.request = self.req_factory.post('unused_url', data={
+ 'password': 'test',
+ 'new_email': self.new_secondary_email
+ })
+ self.request.user = self.user
+ self.user.email_user = Mock()
+
+ def do_email_validation(self, email):
+ """
+ Executes validate_new_secondary_email, returning any resulting error message.
+ """
+ try:
+ validate_new_email(self.request.user, email)
+ except ValueError as err:
+ return text_type(err)
+
+ def do_secondary_email_change(self, user, email, activation_key=None):
+ """
+ Executes do_secondary_email_change_request, returning any resulting error message.
+ """
+ with patch('crum.get_current_request', return_value=self.fake_request):
+ do_email_change_request(
+ user=user,
+ new_email=email,
+ activation_key=activation_key,
+ secondary_email_change_request=True
+ )
+
+ def assertFailedRequest(self, response_data, expected_error):
+ """
+ Assert that `response_data` indicates a failed request that returns `expected_error`
+ """
+ self.assertFalse(response_data['success'])
+ self.assertEquals(expected_error, response_data['error'])
+ self.assertFalse(self.user.email_user.called)
+
+ def test_invalid_emails(self):
+ """
+ Assert the expected error message from the email validation method for an invalid
+ (improperly formatted) email address.
+ """
+ for email in ('bad_email', 'bad_email@', '@bad_email'):
+ self.assertEqual(self.do_email_validation(email), 'Valid e-mail address required.')
+
+ @patch('django.core.mail.send_mail')
+ def test_email_failure(self, send_mail):
+ """
+ Test the return value if sending the email for the user to click fails.
+ """
+ send_mail.side_effect = [Exception, None]
+ with self.assertRaisesRegexp(ValueError, 'Unable to send email activation link. Please try again later.'):
+ self.do_secondary_email_change(self.user, "valid@email.com")
+
+ self.assert_no_events_were_emitted()
+
+ def test_email_success(self):
+ """
+ Test email was sent if no errors encountered.
+ """
+ new_email = "valid@example.com"
+ registration_key = "test-registration-key"
+
+ self.do_secondary_email_change(self.user, new_email, registration_key)
+
+ self._assert_email(
+ subject=u'Request to change édX account secondary e-mail',
+ body_fragments=[
+ u'We received a request to change the secondary e-mail associated with',
+ u'your édX account to {new_email}.'.format(
+ new_email=new_email,
+ ),
+ u'If this is correct, please confirm your new secondary e-mail address by visiting:',
+ u'http://edx.org/activate_secondary_email/{key}'.format(key=registration_key),
+ u'If you didn\'t request this, you don\'t need to do anything;',
+ u'you won\'t receive any more email from us.',
+ u'Please do not reply to this e-mail; if you require assistance,',
+ u'check the help section of the édX web site.',
+ ],
+ )
+
+ def _assert_email(self, subject, body_fragments):
+ """
+ Verify that the email was sent.
+ """
+ assert len(mail.outbox) == 1
+ assert len(body_fragments) > 1, 'Should provide at least two body fragments'
+
+ message = mail.outbox[0]
+ text = message.body
+ html = message.alternatives[0][0]
+
+ assert message.subject == subject
+
+ for body in text, html:
+ for fragment in body_fragments:
+ assert fragment in body
diff --git a/common/djangoapps/student/urls.py b/common/djangoapps/student/urls.py
index 5f0a8d89820cf3c4efa8fbf167ea39db7d61755c..bb5f237e731998ee27d19c6e0ed8ffad03d3d85e 100644
--- a/common/djangoapps/student/urls.py
+++ b/common/djangoapps/student/urls.py
@@ -38,6 +38,13 @@ urlpatterns = [
url(r'^course_run/{}/refund_status$'.format(settings.COURSE_ID_PATTERN),
views.course_run_refund_status,
name="course_run_refund_status"),
+
+ url(
+ r'^activate_secondary_email/(?P<key>[^/]*)$',
+ views.activate_secondary_email,
+ name='activate_secondary_email'
+ ),
+
]
# password reset django views (see above for password reset views)
diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py
index 5a979dc996f8265b22259e5b5899afbfea9ea814..5593f9de6a09dea7fe426358036710fa501f83f8 100644
--- a/common/djangoapps/student/views/management.py
+++ b/common/djangoapps/student/views/management.py
@@ -76,6 +76,7 @@ from student.models import (
AccountRecovery,
CourseEnrollment,
PendingEmailChange,
+ PendingSecondaryEmailChange,
Registration,
RegistrationCookieConfiguration,
UserAttribute,
@@ -1109,26 +1110,35 @@ def validate_secondary_email(account_recovery, new_email):
raise ValueError(message)
-def do_email_change_request(user, new_email, activation_key=None):
+def do_email_change_request(user, new_email, activation_key=None, secondary_email_change_request=False):
"""
Given a new email for a user, does some basic verification of the new address and sends an activation message
to the new address. If any issues are encountered with verification or sending the message, a ValueError will
be thrown.
"""
- pec_list = PendingEmailChange.objects.filter(user=user)
- if len(pec_list) == 0:
- pec = PendingEmailChange()
- pec.user = user
- else:
- pec = pec_list[0]
-
# if activation_key is not passing as an argument, generate a random key
if not activation_key:
activation_key = uuid.uuid4().hex
- pec.new_email = new_email
- pec.activation_key = activation_key
- pec.save()
+ confirm_link = reverse('confirm_email_change', kwargs={'key': activation_key, })
+
+ if secondary_email_change_request:
+ PendingSecondaryEmailChange.objects.update_or_create(
+ user=user,
+ defaults={
+ 'new_secondary_email': new_email,
+ 'activation_key': activation_key,
+ }
+ )
+ confirm_link = reverse('activate_secondary_email', kwargs={'key': activation_key})
+ else:
+ PendingEmailChange.objects.update_or_create(
+ user=user,
+ defaults={
+ 'new_email': new_email,
+ 'activation_key': activation_key,
+ }
+ )
use_https = theming_helpers.get_current_request().is_secure()
@@ -1136,18 +1146,17 @@ def do_email_change_request(user, new_email, activation_key=None):
message_context = get_base_template_context(site)
message_context.update({
'old_email': user.email,
- 'new_email': pec.new_email,
+ 'new_email': new_email,
+ 'is_secondary_email_change_request': secondary_email_change_request,
'confirm_link': '{protocol}://{site}{link}'.format(
protocol='https' if use_https else 'http',
site=configuration_helpers.get_value('SITE_NAME', settings.SITE_NAME),
- link=reverse('confirm_email_change', kwargs={
- 'key': pec.activation_key,
- }),
+ link=confirm_link,
),
})
msg = EmailChange().personalize(
- recipient=Recipient(user.username, pec.new_email),
+ recipient=Recipient(user.username, new_email),
language=preferences_api.get_user_preference(user, LANGUAGE_KEY),
user_context=message_context,
)
@@ -1159,18 +1168,42 @@ def do_email_change_request(user, new_email, activation_key=None):
log.error(u'Unable to send email activation link to user from "%s"', from_address, exc_info=True)
raise ValueError(_('Unable to send email activation link. Please try again later.'))
- # When the email address change is complete, a "edx.user.settings.changed" event will be emitted.
- # But because changing the email address is multi-step, we also emit an event here so that we can
- # track where the request was initiated.
- tracker.emit(
- SETTING_CHANGE_INITIATED,
- {
- "setting": "email",
- "old": message_context['old_email'],
- "new": message_context['new_email'],
- "user_id": user.id,
- }
- )
+ if not secondary_email_change_request:
+ # When the email address change is complete, a "edx.user.settings.changed" event will be emitted.
+ # But because changing the email address is multi-step, we also emit an event here so that we can
+ # track where the request was initiated.
+ tracker.emit(
+ SETTING_CHANGE_INITIATED,
+ {
+ "setting": "email",
+ "old": message_context['old_email'],
+ "new": message_context['new_email'],
+ "user_id": user.id,
+ }
+ )
+
+
+@ensure_csrf_cookie
+def activate_secondary_email(request, key): # pylint: disable=unused-argument
+ """
+ This is called when the activation link is clicked. We activate the secondary email
+ for the requested user.
+ """
+ try:
+ pending_secondary_email_change = PendingSecondaryEmailChange.objects.get(activation_key=key)
+ except PendingSecondaryEmailChange.DoesNotExist:
+ return render_to_response("invalid_email_key.html", {})
+
+ try:
+ account_recovery_obj = AccountRecovery.objects.get(user_id=pending_secondary_email_change.user)
+ except AccountRecovery.DoesNotExist:
+ return render_to_response("secondary_email_change_failed.html", {
+ 'secondary_email': pending_secondary_email_change.new_secondary_email
+ })
+
+ account_recovery_obj.is_active = True
+ account_recovery_obj.save()
+ return render_to_response("secondary_email_change_successful.html")
@ensure_csrf_cookie
diff --git a/common/templates/student/edx_ace/emailchange/email/body.html b/common/templates/student/edx_ace/emailchange/email/body.html
index 93cc842e146fb6c3802394edf51a98389e2995ac..8462c2fa7f339b383dd3cafbe4d829becefc052a 100644
--- a/common/templates/student/edx_ace/emailchange/email/body.html
+++ b/common/templates/student/edx_ace/emailchange/email/body.html
@@ -10,7 +10,11 @@
{% trans "Email Change" %}
</h1>
<p style="color: rgba(0,0,0,.75);">
- {% blocktrans %}We received a request to change the e-mail associated with your {{ platform_name }} account from {{ old_email }} to {{ new_email }}. If this is correct, please confirm your new e-mail address by visiting:{% endblocktrans %}
+ {% if is_secondary_email_change_request %}
+ {% blocktrans %}We received a request to change the secondary e-mail associated with your {{ platform_name }} account to {{ new_email }}. If this is correct, please confirm your new secondary e-mail address by visiting:{% endblocktrans %}
+ {% else %}
+ {% blocktrans %}We received a request to change the e-mail associated with your {{ platform_name }} account from {{ old_email }} to {{ new_email }}. If this is correct, please confirm your new e-mail address by visiting:{% endblocktrans %}
+ {% endif %}
<br />
</p>
diff --git a/common/templates/student/edx_ace/emailchange/email/body.txt b/common/templates/student/edx_ace/emailchange/email/body.txt
index 34c86d59bfdaf66bea89c9fe0ddb15c5d6bd7987..1b9e381748e6a7e7c46cee3244ff276ac850b26e 100644
--- a/common/templates/student/edx_ace/emailchange/email/body.txt
+++ b/common/templates/student/edx_ace/emailchange/email/body.txt
@@ -1,5 +1,9 @@
{% load i18n %}{% autoescape off %}
-{% blocktrans %}We received a request to change the e-mail associated with your {{ platform_name }} account from {{ old_email }} to {{ new_email }}. If this is correct, please confirm your new e-mail address by visiting:{% endblocktrans %}
+{% if is_secondary_email_change_request %}
+ {% blocktrans %}We received a request to change the secondary e-mail associated with your {{ platform_name }} account to {{ new_email }}. If this is correct, please confirm your new secondary e-mail address by visiting:{% endblocktrans %}
+{% else %}
+ {% blocktrans %}We received a request to change the e-mail associated with your {{ platform_name }} account from {{ old_email }} to {{ new_email }}. If this is correct, please confirm your new e-mail address by visiting:{% endblocktrans %}
+{% endif %}
{{ confirm_link }}
diff --git a/common/templates/student/edx_ace/emailchange/email/subject.txt b/common/templates/student/edx_ace/emailchange/email/subject.txt
index 54313b5c8facc18057a267e13b869ec0ba345ed5..ea24a76f50503884939f6adf0a093e7132b01758 100644
--- a/common/templates/student/edx_ace/emailchange/email/subject.txt
+++ b/common/templates/student/edx_ace/emailchange/email/subject.txt
@@ -1,4 +1,8 @@
{% load i18n %}
{% autoescape off %}
-{% blocktrans trimmed %}Request to change {{ platform_name }} account e-mail{% endblocktrans %}
+{% if is_secondary_email_change_request %}
+ {% blocktrans trimmed %}Request to change {{ platform_name }} account secondary e-mail{% endblocktrans %}
+{% else %}
+ {% blocktrans trimmed %}Request to change {{ platform_name }} account e-mail{% endblocktrans %}
+{% endif %}
{% endautoescape %}
diff --git a/lms/static/js/student_account/views/account_settings_factory.js b/lms/static/js/student_account/views/account_settings_factory.js
index 514a35ed0b44007c0ddfe7c6fff7c20a4407c2ee..d8231d9f2ae304677044c53943dd58a92057391a 100644
--- a/lms/static/js/student_account/views/account_settings_factory.js
+++ b/lms/static/js/student_account/views/account_settings_factory.js
@@ -246,7 +246,17 @@
// Secondary email address
if (isSecondaryEmailFeatureEnabled) {
secondaryEmailFieldView = {
- view: new AccountSettingsFieldViews.EmailFieldView(secondaryEmailFieldData)
+ view: new AccountSettingsFieldViews.EmailFieldView(secondaryEmailFieldData),
+ successMessage: function() {
+ return HtmlUtils.joinHtml(
+ this.indicators.success,
+ StringUtils.interpolate(
+ gettext('We\'ve sent a confirmation message to {new_secondary_email_address}. Click the link in the message to update your secondary email address.'), // eslint-disable-line max-len
+ {
+ new_secondary_email_address: this.fieldValue()
+ }
+ )
+ );}
};
emailFieldViewIndex = aboutSectionsData[0].fields.indexOf(emailFieldView);
diff --git a/lms/templates/secondary_email_change_failed.html b/lms/templates/secondary_email_change_failed.html
new file mode 100644
index 0000000000000000000000000000000000000000..cff94c902046edee631b6bc85b2fc4673fb43dbc
--- /dev/null
+++ b/lms/templates/secondary_email_change_failed.html
@@ -0,0 +1,20 @@
+## xss-lint: disable=mako-missing-default,python-wrap-html
+<%! from django.utils.translation import ugettext as _ %>
+
+<%inherit file="main.html" />
+
+<section class="container activation">
+
+ <section class="message">
+ <h1 class="invalid">${_("Secondary e-mail change failed")}</h1>
+ <hr class="horizontal-divider">
+
+ % if err_msg:
+ <p>${err_msg}</p>
+ % else:
+ <p>${_("We were unable to activate your secondary email {secondary_email}").format(secondary_email=secondary_email)}</p>
+ % endif
+ ## xss-lint: disable=python-wrap-html
+ <p>${_('Go back to the {link_start}home page{link_end}.').format(link_start='<a href="/">', link_end='</a>')}</p>
+ </section>
+</section>
diff --git a/lms/templates/secondary_email_change_successful.html b/lms/templates/secondary_email_change_successful.html
new file mode 100644
index 0000000000000000000000000000000000000000..0d8d8a0b6efc208d9a7219b1aa68af15fc2ee3f5
--- /dev/null
+++ b/lms/templates/secondary_email_change_successful.html
@@ -0,0 +1,20 @@
+## xss-lint: disable=mako-missing-default
+<%inherit file="main.html" />
+<%!
+from django.utils.translation import ugettext as _
+from django.urls import reverse
+from openedx.core.djangolib.markup import HTML, Text
+
+%>
+
+<section class="container activation">
+ <section class="message">
+ <h1 class="valid">${_("Secondary e-mail change successful!")}</h1>
+ <hr class="horizontal-divider">
+
+ <p>${Text(_('Your secondary email has been activated. Please visit {link_start}dashboard{link_end} for courses.')).format(
+ link_start=HTML('<a href="{url}">').format(url=reverse('dashboard')),
+ link_end=HTML('</a>'),
+ )}</p>
+ </section>
+</section>
diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py
index 3bc667eaafef95def7cdac3ce26f8d094e5b76fc..296e94d033f002aba054054730e4448148ba8c98 100644
--- a/openedx/core/djangoapps/user_api/accounts/api.py
+++ b/openedx/core/djangoapps/user_api/accounts/api.py
@@ -299,6 +299,18 @@ def update_account_settings(requesting_user, update, username=None):
u"Error thrown from do_email_change_request: '{}'".format(text_type(err)),
user_message=text_type(err)
)
+ if changing_secondary_email:
+ try:
+ student_views.do_email_change_request(
+ user=existing_user,
+ new_email=update["secondary_email"],
+ secondary_email_change_request=True,
+ )
+ except ValueError as err:
+ raise AccountUpdateError(
+ u"Error thrown from do_email_change_request: '{}'".format(text_type(err)),
+ user_message=text_type(err)
+ )
@helpers.intercept_errors(errors.UserAPIInternalError, ignore_errors=[errors.UserAPIRequestError])