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

edX-ACE support for email change messages

Browse Source
This commit is contained in:
Omar Al-Ithawi
2018-09-05 18:17:12 +03:00
parent f9488a8502
commit ceacfc8a75
10 changed files with 133 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

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

80
common/djangoapps/student/tests/test_email.py
View File

@@ -1,4 +1,4 @@
# coding=utf-8
import json
import unittest
@@ -14,8 +14,10 @@ from mock import Mock, patch
from six import text_type
from edxmako.shortcuts import render_to_string
from openedx.core.djangoapps.ace_common.tests.mixins import EmailTemplateTagMixin
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
from openedx.core.djangoapps.theming.tests.test_util import with_comprehensive_theme
from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration
from openedx.core.djangoapps.user_api.config.waffle import PREVENT_AUTH_USER_WRITES, SYSTEM_MAINTENANCE_MSG, waffle
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, CacheIsolationMixin
from student.models import (
@@ -275,13 +277,14 @@ class ReactivationEmailTests(EmailTestMixin, CacheIsolationTestCase):
self.assertTrue(response_data['success'])
class EmailChangeRequestTests(EventTestMixin, CacheIsolationTestCase):
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', "Test only valid in LMS")
class EmailChangeRequestTests(EventTestMixin, EmailTemplateTagMixin, CacheIsolationTestCase):
"""
Test changing a user's email address
"""
def setUp(self):
super(EmailChangeRequestTests, self).setUp('student.views.management.tracker')
def setUp(self, tracker='student.views.management.tracker'):
super(EmailChangeRequestTests, self).setUp(tracker)
self.user = UserFactory.create()
self.new_email = 'new.email@edx.org'
self.req_factory = RequestFactory()
@@ -305,10 +308,8 @@ class EmailChangeRequestTests(EventTestMixin, CacheIsolationTestCase):
"""
Executes do_email_change_request, returning any resulting error message.
"""
try:
with patch('crum.get_current_request', return_value=self.fake_request):
do_email_change_request(user, email, activation_key)
except ValueError as err:
return text_type(err)
def assertFailedRequest(self, response_data, expected_error):
"""
@@ -332,8 +333,8 @@ class EmailChangeRequestTests(EventTestMixin, CacheIsolationTestCase):
user2 = UserFactory.create(email=self.new_email, password="test2")
# Send requests & ensure no error was thrown
self.assertIsNone(self.do_email_change(self.user, user1_new_email))
self.assertIsNone(self.do_email_change(user2, user2_new_email))
self.do_email_change(self.user, user1_new_email)
self.do_email_change(user2, user2_new_email)
def test_invalid_emails(self):
"""
@@ -350,43 +351,64 @@ class EmailChangeRequestTests(EventTestMixin, CacheIsolationTestCase):
self.assertEqual(self.do_email_validation(self.user.email), 'Old email is the same as the new email.')
@patch('django.core.mail.send_mail')
@patch('student.views.management.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True))
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]
self.assertEqual(
self.do_email_change(self.user, "valid@email.com"),
'Unable to send email activation link. Please try again later.'
)
with self.assertRaisesRegexp(ValueError, 'Unable to send email activation link. Please try again later.'):
self.do_email_change(self.user, "valid@email.com")
self.assert_no_events_were_emitted()
@patch('django.core.mail.send_mail')
@patch('student.views.management.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True))
def test_email_success(self, send_mail):
def test_email_success(self):
"""
Test email was sent if no errors encountered.
"""
old_email = self.user.email
new_email = "valid@example.com"
registration_key = "test registration key"
self.assertIsNone(self.do_email_change(self.user, new_email, registration_key))
context = {
'key': registration_key,
'old_email': old_email,
'new_email': new_email
}
send_mail.assert_called_with(
mock_render_to_string('emails/email_change_subject.txt', context),
mock_render_to_string('emails/email_change.txt', context),
configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL),
[new_email]
registration_key = "test-registration-key"
self.do_email_change(self.user, new_email, registration_key)
self._assert_email(
subject=u'Request to change édX account e-mail',
body_fragments=[
u'We received a request to change the e-mail associated with',
u'your édX account from {old_email} to {new_email}.'.format(
old_email=old_email,
new_email=new_email,
),
u'If this is correct, please confirm your new e-mail address by visiting:',
u'http://edx.org/email_confirm/{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.',
],
)
self.assert_event_emitted(
SETTING_CHANGE_INITIATED, user_id=self.user.id, setting=u'email', old=old_email, new=new_email
)
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
@patch('django.contrib.auth.models.User.email_user')
@patch('student.views.management.render_to_response', Mock(side_effect=mock_render_to_response, autospec=True))

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

@@ -14,6 +14,7 @@ from django.contrib import messages
from django.contrib.auth.decorators import login_required
from django.contrib.auth.models import AnonymousUser, User
from django.contrib.auth.views import password_reset_confirm
from django.contrib.sites.models import Site
from django.core import mail
from django.urls import reverse
from django.core.validators import ValidationError, validate_email
@@ -29,6 +30,8 @@ from django.utils.http import base36_to_int, urlsafe_base64_encode
from django.utils.translation import ugettext as _
from django.views.decorators.csrf import csrf_exempt, ensure_csrf_cookie
from django.views.decorators.http import require_GET, require_POST, require_http_methods
from edx_ace import ace
from edx_ace.recipient import Recipient
from edx_django_utils import monitoring as monitoring_utils
from eventtracking import tracker
from ipware.ip import get_ip
@@ -38,7 +41,6 @@ from opaque_keys.edx.keys import CourseKey
from pytz import UTC
from six import text_type
from xmodule.modulestore.django import modulestore
import track.views
from course_modes.models import CourseMode
from edx_ace import ace
@@ -49,6 +51,7 @@ from entitlements.models import CourseEntitlement
from openedx.core.djangoapps.ace_common.template_context import get_base_template_context
from openedx.core.djangoapps.catalog.utils import get_programs_with_type
from openedx.core.djangoapps.embargo import api as embargo_api
from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY
from openedx.core.djangoapps.oauth_dispatch.api import destroy_oauth_tokens
from openedx.core.djangoapps.programs.models import ProgramsApiConfig
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
@@ -71,7 +74,7 @@ from student.helpers import (
generate_activation_email_context,
get_next_url_for_login_page
)
from student.message_types import PasswordReset
from student.message_types import EmailChange, PasswordReset
from student.models import (
CourseEnrollment,
PasswordHistory,
@@ -920,24 +923,32 @@ def do_email_change_request(user, new_email, activation_key=None):
pec.activation_key = activation_key
pec.save()
context = {
'key': pec.activation_key,
use_https = theming_helpers.get_current_request().is_secure()
site = Site.objects.get_current()
message_context = get_base_template_context(site)
message_context.update({
'old_email': user.email,
'new_email': pec.new_email
}
'new_email': pec.new_email,
'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,
}),
),
})
subject = render_to_string('emails/email_change_subject.txt', context)
subject = ''.join(subject.splitlines())
message = render_to_string('emails/email_change.txt', context)
from_address = configuration_helpers.get_value(
'email_from_address',
settings.DEFAULT_FROM_EMAIL
msg = EmailChange().personalize(
recipient=Recipient(user.username, pec.new_email),
language=preferences_api.get_user_preference(user, LANGUAGE_KEY),
user_context=message_context,
)
try:
mail.send_mail(subject, message, from_address, [pec.new_email])
ace.send(msg)
except Exception:
from_address = configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL)
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.'))
@@ -948,8 +959,8 @@ def do_email_change_request(user, new_email, activation_key=None):
SETTING_CHANGE_INITIATED,
{
"setting": "email",
"old": context['old_email'],
"new": context['new_email'],
"old": message_context['old_email'],
"new": message_context['new_email'],
"user_id": user.id,
}
)

28
common/templates/student/edx_ace/emailchange/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 "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 %}
<br />
</p>
{% trans "Confirm Email Change" 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=confirm_link %}
<p style="color: rgba(0,0,0,.75);">
{% blocktrans %}If you didn't request this, you don't need to do anything; you won't receive any more email from us. Please do not reply to this e-mail; if you require assistance, check the help section of the {{ platform_name }} web site.{% endblocktrans %}
<br />
</p>
</td>
</tr>
</table>
{% endblock %}

6
common/templates/student/edx_ace/emailchange/email/body.txt Normal file
View File

@@ -0,0 +1,6 @@
{% 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 %}
{{ confirm_link }}
{% blocktrans %}If you didn't request this, you don't need to do anything; you won't receive any more email from us. Please do not reply to this e-mail; if you require assistance, check the help section of the {{ platform_name }} web site.{% endblocktrans %}{% endautoescape %}

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

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

1
common/templates/student/edx_ace/emailchange/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/emailchange/email/subject.txt Normal file
View File

@@ -0,0 +1,4 @@
{% load i18n %}
{% autoescape off %}
{% blocktrans trimmed %}Request to change {{ platform_name }} account e-mail{% endblocktrans %}
{% endautoescape %}

23
lms/templates/emails/email_change.txt
View File

@@ -1,23 +0,0 @@
## mako
<%! from django.utils.translation import ugettext as _ %>
<%! from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers %>
${_("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:").format(
platform_name=configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME),
old_email=old_email,
new_email=new_email
)
}
% if is_secure:
https://${ site }/email_confirm/${ key }
% else:
http://${ site }/email_confirm/${ key }
% endif
${_("If you didn't request this, you don't need to do anything; you won't "
"receive any more email from us. Please do not reply to this e-mail; "
"if you require assistance, check the help section of the "
"{platform_name} web site.").format(platform_name=configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME))}

10
openedx/core/djangoapps/user_api/accounts/tests/test_api.py
View File

@@ -17,6 +17,7 @@ from django.test.client import RequestFactory
from mock import Mock, patch
from six import iteritems
from openedx.core.djangoapps.ace_common.tests.mixins import EmailTemplateTagMixin
from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory
from openedx.core.djangoapps.user_api.accounts import PRIVATE_VISIBILITY, USERNAME_MAX_LENGTH
from openedx.core.djangoapps.user_api.accounts.api import (
@@ -64,7 +65,7 @@ def mock_render_to_string(template_name, context):
@attr(shard=2)
@skip_unless_lms
class TestAccountApi(UserSettingsEventTestMixin, RetirementTestCase):
class TestAccountApi(UserSettingsEventTestMixin, EmailTemplateTagMixin, RetirementTestCase):
"""
These tests specifically cover the parts of the API methods that are not covered by test_views.py.
This includes the specific types of error raised, and default behavior when optional arguments
@@ -215,8 +216,10 @@ class TestAccountApi(UserSettingsEventTestMixin, RetirementTestCase):
"name": "Mickey Mouse",
"email": "seems_ok@sample.com"
}
with self.assertRaises(AccountUpdateError) as context_manager:
update_account_settings(self.user, less_naughty_update)
with patch('crum.get_current_request', return_value=self.fake_request):
with self.assertRaises(AccountUpdateError) as context_manager:
update_account_settings(self.user, less_naughty_update)
self.assertIn("Error thrown from do_email_change_request", context_manager.exception.developer_message)
# Verify that the name change happened, even though the attempt to send the email failed.
@@ -229,6 +232,7 @@ class TestAccountApi(UserSettingsEventTestMixin, RetirementTestCase):
Test that email address changes are rejected when ALLOW_EMAIL_ADDRESS_CHANGE is not set.
"""
disabled_update = {"email": "valid@example.com"}
with self.assertRaises(AccountUpdateError) as context_manager:
update_account_settings(self.user, disabled_update)
self.assertIn("Email address changes have been disabled", context_manager.exception.developer_message)