diff --git a/cms/templates/emails/activation_email.txt b/cms/templates/emails/activation_email.txt new file mode 100644 index 0000000000..7a54fc6b35 --- /dev/null +++ b/cms/templates/emails/activation_email.txt @@ -0,0 +1,11 @@ +<%! from django.utils.translation import ugettext as _ %> + +${_("Thank you for signing up for {studio_name}! To activate your account, please copy and paste this address into your web browser's address bar:").format(studio_name=settings.STUDIO_NAME)} + +% if is_secure: + https://${ site }/activate/${ key } +% else: + http://${ site }/activate/${ 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 {studio_name} web site.").format(studio_name=settings.STUDIO_NAME)} diff --git a/cms/templates/emails/activation_email_subject.txt b/cms/templates/emails/activation_email_subject.txt new file mode 100644 index 0000000000..d638865710 --- /dev/null +++ b/cms/templates/emails/activation_email_subject.txt @@ -0,0 +1,2 @@ +<%! from django.utils.translation import ugettext as _ %> +${_("Your account for {studio_name}").format(studio_name=settings.STUDIO_NAME)} diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index 7ed9fb2a04..366d6d33d5 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -381,9 +381,7 @@ def generate_activation_email_context(user, registration): 'key': registration.activation_key, 'lms_url': configuration_helpers.get_value('LMS_ROOT_URL', settings.LMS_ROOT_URL), 'platform_name': configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME), - 'support_url': configuration_helpers.get_value( - 'ACTIVATION_EMAIL_SUPPORT_LINK', settings.ACTIVATION_EMAIL_SUPPORT_LINK - ) or settings.SUPPORT_SITE_LINK, + 'support_url': configuration_helpers.get_value('SUPPORT_SITE_LINK', settings.SUPPORT_SITE_LINK), 'support_email': configuration_helpers.get_value('CONTACT_EMAIL', settings.CONTACT_EMAIL), } diff --git a/common/djangoapps/student/message_types.py b/common/djangoapps/student/message_types.py index 681ecfacb5..331197f059 100644 --- a/common/djangoapps/student/message_types.py +++ b/common/djangoapps/student/message_types.py @@ -40,10 +40,3 @@ class RecoveryEmailCreate(BaseMessageType): super(RecoveryEmailCreate, self).__init__(*args, **kwargs) self.options['transactional'] = True - - -class AccountActivation(BaseMessageType): - def __init__(self, *args, **kwargs): - super(AccountActivation, self).__init__(*args, **kwargs) - - self.options['transactional'] = True diff --git a/common/djangoapps/student/tasks.py b/common/djangoapps/student/tasks.py index 73b02a6261..ed32d952ad 100644 --- a/common/djangoapps/student/tasks.py +++ b/common/djangoapps/student/tasks.py @@ -5,43 +5,30 @@ from __future__ import absolute_import import logging +from boto.exception import NoAuthHandlerFound from celery.exceptions import MaxRetriesExceededError from celery.task import task # pylint: disable=no-name-in-module, import-error from django.conf import settings -from edx_ace import ace -from edx_ace.errors import RecoverableChannelDeliveryError -from edx_ace.message import Message -from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers +from django.core import mail log = logging.getLogger('edx.celery.task') @task(bind=True) -def send_activation_email(self, msg_string, from_address=None): +def send_activation_email(self, subject, message, from_address, dest_addr): """ Sending an activation email to the user. """ - msg = Message.from_string(msg_string) - max_retries = settings.RETRY_ACTIVATION_EMAIL_MAX_ATTEMPTS retries = self.request.retries - - if from_address is None: - from_address = configuration_helpers.get_value('ACTIVATION_EMAIL_FROM_ADDRESS') or ( - configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL) - ) - msg.options['from_address'] = from_address - - dest_addr = msg.recipient.email_address - try: - ace.send(msg) + mail.send_mail(subject, message, from_address, [dest_addr], fail_silently=False) # Log that the Activation Email has been sent to user without an exception log.info("Activation Email has been sent to User {user_email}".format( user_email=dest_addr )) - except RecoverableChannelDeliveryError: - log.info('Retrying sending email to user {dest_addr}, attempt # {attempt} of {max_attempts}'.format( + except NoAuthHandlerFound: # pylint: disable=broad-except + log.info('Retrying sending email to user {dest_addr}, attempt # {attempt} of {max_attempts}'. format( dest_addr=dest_addr, attempt=retries, max_attempts=max_retries diff --git a/common/djangoapps/student/tests/test_email.py b/common/djangoapps/student/tests/test_email.py index 7e0137dd84..5089596d90 100644 --- a/common/djangoapps/student/tests/test_email.py +++ b/common/djangoapps/student/tests/test_email.py @@ -25,12 +25,13 @@ from openedx.core.djangoapps.user_api.config.waffle import PREVENT_AUTH_USER_WRI from openedx.core.djangolib.testing.utils import CacheIsolationMixin, CacheIsolationTestCase from openedx.core.lib.request_utils import safe_get_host from student.models import PendingEmailChange, Registration, UserProfile -from student.tests.factories import PendingEmailChangeFactory, UserFactory +from student.tests.factories import PendingEmailChangeFactory, RegistrationFactory, UserFactory from student.views import ( SETTING_CHANGE_INITIATED, confirm_email_change, do_email_change_request, generate_activation_email_context, + send_reactivation_email_for_user, validate_new_email ) from third_party_auth.views import inactive_user_view @@ -90,9 +91,8 @@ class EmailTestMixin(object): self.addCleanup(settings.ALLOWED_HOSTS.pop) -@ddt.ddt @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') -class ActivationEmailTests(EmailTemplateTagMixin, CacheIsolationTestCase): +class ActivationEmailTests(CacheIsolationTestCase): """ Test sending of the activation email. """ @@ -102,36 +102,22 @@ class ActivationEmailTests(EmailTemplateTagMixin, CacheIsolationTestCase): # Text fragments we expect in the body of an email # sent from an OpenEdX installation. OPENEDX_FRAGMENTS = [ + u"high-quality {platform} courses".format(platform=settings.PLATFORM_NAME), + "http://edx.org/activate/", ( - u"Use the link below to activate your account to access engaging, " - u"high-quality {platform_name} courses. Note that you will not be able to log back into your " - u"account until you have activated it.".format( - platform_name=settings.PLATFORM_NAME - ) - ), - u"{}/activate/".format(settings.LMS_ROOT_URL), - u"If you need help, please use our web form at ", ( - settings.ACTIVATION_EMAIL_SUPPORT_LINK or settings.SUPPORT_SITE_LINK - ), - settings.CONTACT_EMAIL, - u"This email message was automatically sent by ", - settings.LMS_ROOT_URL, - u" because someone attempted to create an account on {platform_name}".format( - platform_name=settings.PLATFORM_NAME - ), - u" using this email address." + u"please use our web form at " + u"{support_url} ".format(support_url=settings.SUPPORT_SITE_LINK) + ) ] - @ddt.data('plain_text', 'html') - def test_activation_email(self, test_body_type): + def test_activation_email(self): self._create_account() - self._assert_activation_email(self.ACTIVATION_SUBJECT, self.OPENEDX_FRAGMENTS, test_body_type) + self._assert_activation_email(self.ACTIVATION_SUBJECT, self.OPENEDX_FRAGMENTS) @with_comprehensive_theme("edx.org") - @ddt.data('plain_text', 'html') - def test_activation_email_edx_domain(self, test_body_type): + def test_activation_email_edx_domain(self): self._create_account() - self._assert_activation_email(self.ACTIVATION_SUBJECT, self.OPENEDX_FRAGMENTS, test_body_type) + self._assert_activation_email(self.ACTIVATION_SUBJECT, self.OPENEDX_FRAGMENTS) def _create_account(self): """ @@ -155,23 +141,15 @@ class ActivationEmailTests(EmailTemplateTagMixin, CacheIsolationTestCase): ) ) - def _assert_activation_email(self, subject, body_fragments, test_body_type): + def _assert_activation_email(self, subject, body_fragments): """ Verify that the activation email was sent. """ self.assertEqual(len(mail.outbox), 1) msg = mail.outbox[0] self.assertEqual(msg.subject, subject) - - body_text = { - 'plain_text': msg.body, - 'html': msg.alternatives[0][0] - } - assert test_body_type in body_text - body_to_be_tested = body_text[test_body_type] - for fragment in body_fragments: - self.assertIn(fragment, body_to_be_tested) + self.assertIn(fragment, msg.body) def test_do_not_send_email_and_do_activate(self): """ @@ -219,6 +197,85 @@ class ActivationEmailTests(EmailTemplateTagMixin, CacheIsolationTestCase): ) +@patch('student.views.management.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True)) +@patch('django.contrib.auth.models.User.email_user') +class ReactivationEmailTests(EmailTestMixin, CacheIsolationTestCase): + """ + Test sending a reactivation email to a user + """ + + def setUp(self): + super(ReactivationEmailTests, self).setUp() + self.user = UserFactory.create() + self.unregisteredUser = UserFactory.create() + self.registration = RegistrationFactory.create(user=self.user) + + def reactivation_email(self, user): + """ + Send the reactivation email to the specified user, + and return the response as json data. + """ + return json.loads(send_reactivation_email_for_user(user).content.decode('utf-8')) + + def assertReactivateEmailSent(self, email_user): + """ + Assert that the correct reactivation email has been sent + """ + context = generate_activation_email_context(self.user, self.registration) + + self.assertEmailUser( + email_user, + 'emails/activation_email_subject.txt', + context, + 'emails/activation_email.txt', + context + ) + + # Thorough tests for safe_get_host are elsewhere; here we just want a quick URL sanity check + request = RequestFactory().post('unused_url') + request.user = self.user + request.META['HTTP_HOST'] = "aGenericValidHostName" + self.append_allowed_hosts("aGenericValidHostName") + + with patch('edxmako.request_context.get_current_request', return_value=request): + body = render_to_string('emails/activation_email.txt', context) + host = safe_get_host(request) + + self.assertIn(host, body) + + def test_reactivation_email_failure(self, email_user): + self.user.email_user.side_effect = Exception + response_data = self.reactivation_email(self.user) + + self.assertReactivateEmailSent(email_user) + self.assertFalse(response_data['success']) + + def test_reactivation_for_unregistered_user(self, email_user): # pylint: disable=unused-argument + """ + Test that trying to send a reactivation email to an unregistered + user fails without throwing a 500 error. + """ + response_data = self.reactivation_email(self.unregisteredUser) + + self.assertFalse(response_data['success']) + + def test_reactivation_for_no_user_profile(self, email_user): # pylint: disable=unused-argument + """ + Test that trying to send a reactivation email to a user without + user profile fails without throwing 500 error. + """ + user = UserFactory.build(username='test_user', email='test_user@test.com') + user.save() + response_data = self.reactivation_email(user) + self.assertFalse(response_data['success']) + + def test_reactivation_email_success(self, email_user): + response_data = self.reactivation_email(self.user) + + self.assertReactivateEmailSent(email_user) + self.assertTrue(response_data['success']) + + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', "Test only valid in LMS") class EmailChangeRequestTests(EventTestMixin, EmailTemplateTagMixin, CacheIsolationTestCase): """ diff --git a/common/djangoapps/student/tests/test_tasks.py b/common/djangoapps/student/tests/test_tasks.py index bbebedc629..a377aea952 100644 --- a/common/djangoapps/student/tests/test_tasks.py +++ b/common/djangoapps/student/tests/test_tasks.py @@ -5,15 +5,13 @@ Tests for the Sending activation email celery tasks from __future__ import absolute_import import mock +from boto.exception import NoAuthHandlerFound from django.conf import settings from django.test import TestCase from six.moves import range -from edx_ace.errors import ChannelError, RecoverableChannelDeliveryError from lms.djangoapps.courseware.tests.factories import UserFactory -from student.models import Registration from student.tasks import send_activation_email -from student.views.management import compose_activation_email class SendActivationEmailTestCase(TestCase): @@ -25,32 +23,18 @@ class SendActivationEmailTestCase(TestCase): super(SendActivationEmailTestCase, self).setUp() self.student = UserFactory() - registration = Registration() - registration.register(self.student) - - self.msg = compose_activation_email("http://www.example.com", self.student, registration) - - def test_ComposeEmail(self): - """ - Tests that attributes of the message are being filled correctly in compose_activation_email - """ - self.assertEquals(self.msg.recipient.username, self.student.username) - self.assertEquals(self.msg.recipient.email_address, self.student.email) - self.assertEquals(self.msg.context['routed_user'], self.student.username) - self.assertEquals(self.msg.context['routed_user_email'], self.student.email) - self.assertEquals(self.msg.context['routed_profile_name'], '') - @mock.patch('time.sleep', mock.Mock(return_value=None)) @mock.patch('student.tasks.log') - @mock.patch('student.tasks.ace.send', mock.Mock(side_effect=RecoverableChannelDeliveryError(None, None))) - def test_RetrySendUntilFail(self, mock_log): + @mock.patch('django.core.mail.send_mail', mock.Mock(side_effect=NoAuthHandlerFound)) + def test_send_email(self, mock_log): """ Tests retries when the activation email doesn't send """ from_address = 'task_testing@example.com' email_max_attempts = settings.RETRY_ACTIVATION_EMAIL_MAX_ATTEMPTS - send_activation_email.delay(str(self.msg), from_address=from_address) + # pylint: disable=no-member + send_activation_email.delay('Task_test', 'Task_test_message', from_address, self.student.email) # Asserts sending email retry logging. for attempt in range(email_max_attempts): @@ -70,26 +54,3 @@ class SendActivationEmailTestCase(TestCase): exc_info=True ) self.assertEquals(mock_log.error.call_count, 1) - - @mock.patch('student.tasks.log') - @mock.patch('student.tasks.ace.send', mock.Mock(side_effect=ChannelError)) - def test_UnrecoverableSendError(self, mock_log): - """ - Tests that a major failure of the send is logged - """ - from_address = 'task_testing@example.com' - - send_activation_email.delay(str(self.msg), from_address=from_address) - - # Asserts that the error was logged - mock_log.exception.assert_called_with( - 'Unable to send activation email to user from "%s" to "%s"', - from_address, - self.student.email, - exc_info=True - ) - - # Assert that nothing else was logged - self.assertEquals(mock_log.info.call_count, 0) - self.assertEquals(mock_log.error.call_count, 0) - self.assertEquals(mock_log.exception.call_count, 1) diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 523fa444b0..711e766d9a 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -66,13 +66,7 @@ from openedx.core.djangoapps.user_api.preferences import api as preferences_api from openedx.core.djangolib.markup import HTML, Text from student.forms import AccountCreationForm, PasswordResetFormNoActive, get_registration_extension_form from student.helpers import DISABLE_UNENROLL_CERT_STATES, cert_info, generate_activation_email_context -from student.message_types import ( - AccountActivation, - EmailChange, - EmailChangeConfirmation, - PasswordReset, - RecoveryEmailCreate -) +from student.message_types import EmailChange, EmailChangeConfirmation, PasswordReset, RecoveryEmailCreate from student.models import ( AccountRecovery, CourseEnrollment, @@ -184,40 +178,6 @@ def index(request, extra_context=None, user=AnonymousUser()): return render_to_response('index.html', context) -def compose_activation_email(root_url, user, user_registration=None, route_enabled=False, profile_name=''): - """ - Construct all the required params for the activation email - through celery task - """ - if user_registration is None: - user_registration = Registration.objects.get(user=user) - - message_context = generate_activation_email_context(user, user_registration) - message_context.update({ - 'confirm_activation_link': '{root_url}/activate/{activation_key}'.format( - root_url=root_url, - activation_key=message_context['key'] - ), - 'route_enabled': route_enabled, - 'routed_user': user.username, - 'routed_user_email': user.email, - 'routed_profile_name': profile_name, - }) - - if route_enabled: - dest_addr = settings.FEATURES['REROUTE_ACTIVATION_EMAIL'] - else: - dest_addr = user.email - - msg = AccountActivation().personalize( - recipient=Recipient(user.username, dest_addr), - language=preferences_api.get_user_preference(user, LANGUAGE_KEY), - user_context=message_context, - ) - - return msg - - def compose_and_send_activation_email(user, profile, user_registration=None): """ Construct all the required params and send the activation email @@ -228,12 +188,66 @@ def compose_and_send_activation_email(user, profile, user_registration=None): profile: profile object of the current logged-in user user_registration: registration of the current logged-in user """ - route_enabled = settings.FEATURES.get('REROUTE_ACTIVATION_EMAIL') + dest_addr = user.email + if user_registration is None: + user_registration = Registration.objects.get(user=user) + context = generate_activation_email_context(user, user_registration) + subject = render_to_string('emails/activation_email_subject.txt', context) + # Email subject *must not* contain newlines + subject = ''.join(subject.splitlines()) + message_for_activation = render_to_string('emails/activation_email.txt', context) + from_address = configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL) + from_address = configuration_helpers.get_value('ACTIVATION_EMAIL_FROM_ADDRESS', from_address) + if settings.FEATURES.get('REROUTE_ACTIVATION_EMAIL'): + dest_addr = settings.FEATURES['REROUTE_ACTIVATION_EMAIL'] + message_for_activation = ("Activation for %s (%s): %s\n" % (user, user.email, profile.name) + + '-' * 80 + '\n\n' + message_for_activation) + send_activation_email.delay(subject, message_for_activation, from_address, dest_addr) - root_url = configuration_helpers.get_value('LMS_ROOT_URL', settings.LMS_ROOT_URL) - msg = compose_activation_email(root_url, user, user_registration, route_enabled, profile.name) - send_activation_email.delay(str(msg)) +def send_reactivation_email_for_user(user): + try: + registration = Registration.objects.get(user=user) + except Registration.DoesNotExist: + return JsonResponse({ + "success": False, + "error": _('No inactive user with this e-mail exists'), + }) + + try: + context = generate_activation_email_context(user, registration) + except ObjectDoesNotExist: + log.error( + u'Unable to send reactivation email due to unavailable profile for the user "%s"', + user.username, + exc_info=True + ) + return JsonResponse({ + "success": False, + "error": _('Unable to send reactivation email') + }) + + subject = render_to_string('emails/activation_email_subject.txt', context) + subject = ''.join(subject.splitlines()) + message = render_to_string('emails/activation_email.txt', context) + from_address = configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL) + from_address = configuration_helpers.get_value('ACTIVATION_EMAIL_FROM_ADDRESS', from_address) + + try: + user.email_user(subject, message, from_address) + except Exception: # pylint: disable=broad-except + log.error( + u'Unable to send reactivation email from "%s" to "%s"', + from_address, + user.email, + exc_info=True + ) + return JsonResponse({ + "success": False, + "error": _('Unable to send reactivation email') + }) + + return JsonResponse({"success": True}) @login_required @@ -537,9 +551,7 @@ def activate_account(request, key): '{html_start}Your account could not be activated{html_end}' 'Something went wrong, please contact support to resolve this issue.' )).format( - support_url=configuration_helpers.get_value( - 'ACTIVATION_EMAIL_SUPPORT_LINK', settings.ACTIVATION_EMAIL_SUPPORT_LINK - ) or settings.SUPPORT_SITE_LINK, + support_url=configuration_helpers.get_value('SUPPORT_SITE_LINK', settings.SUPPORT_SITE_LINK), html_start=HTML('
'), ), diff --git a/common/templates/student/edx_ace/accountactivation/email/body.html b/common/templates/student/edx_ace/accountactivation/email/body.html deleted file mode 100644 index d6970cbba9..0000000000 --- a/common/templates/student/edx_ace/accountactivation/email/body.html +++ /dev/null @@ -1,69 +0,0 @@ -{% extends 'ace_common/edx_ace/common/base_body.html' %} - -{% load django_markup %} -{% load i18n %} -{% load static %} -{% block content %} -|
-
- {% filter force_escape %}
- {% blocktrans %}This is a routed Account Activation email for {{ routed_user }} ({{ routed_user_email }}): {{ routed_profile_name }}{% endblocktrans %}
- {% endfilter %}
- |
-
- - {% trans "Account Activation" as header_msg %}{{ header_msg | force_escape }} --
- {% filter force_escape %}
- {% blocktrans %}You're almost there! Use the link below to activate your account to access engaging, high-quality {{ platform_name }} courses. Note that you will not be able to log back into your account until you have activated it.{% endblocktrans %}
- {% endfilter %}
- |
-
|
-
- {% filter force_escape %}
- {% blocktrans %}Enjoy learning with {{ platform_name }}.{% endblocktrans %}
- {% endfilter %}
- |
-
-
- {% blocktrans trimmed asvar assist_msg %}
- If you need help, please use our web form at {start_anchor_web}{{ support_url }}{end_anchor} or email {start_anchor_email}{{ support_email }}{end_anchor}.
- {% endblocktrans %}
- {% interpolate_html assist_msg start_anchor_web=''|safe start_anchor_email=''|safe end_anchor=''|safe %}
- |
-
|
-
- {% filter force_escape %}
- {% blocktrans %}This email message was automatically sent by {{ lms_url }} because someone attempted to create an account on {{ platform_name }} using this email address.{% endblocktrans %}
- {% endfilter %}
- |
-