Skip to content
Snippets Groups Projects
Unverified Commit 73af914c authored by Omar Al-Ithawi's avatar Omar Al-Ithawi Committed by Gabe Mulley
Browse files

Use edx-ace for password reset email

parent 93809a84
Branches
Tags
No related merge requests found
Showing
with 242 additions and 109 deletions
......@@ -27,7 +27,8 @@ for pkg_name in ['track.contexts', 'track.middleware', 'dd.dogapi']:
################################ EMAIL ########################################
EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend'
EMAIL_BACKEND = 'django.core.mail.backends.filebased.EmailBackend'
EMAIL_FILE_PATH = '/edx/src/ace_messages/'
################################# LMS INTEGRATION #############################
......
......@@ -10,15 +10,22 @@ from django.contrib.auth.forms import PasswordResetForm
from django.contrib.auth.hashers import UNUSABLE_PASSWORD_PREFIX
from django.contrib.auth.models import User
from django.contrib.auth.tokens import default_token_generator
from django.contrib.sites.models import Site
from django.core.exceptions import ValidationError
from django.core.urlresolvers import reverse
from django.core.validators import RegexValidator, slug_re
from django.forms import widgets
from django.template import loader
from django.utils.http import int_to_base36
from django.utils.translation import ugettext_lazy as _
from django.core.validators import RegexValidator, slug_re
from edx_ace import ace
from edx_ace.recipient import Recipient
from openedx.core.djangoapps.ace_common.template_context import get_base_template_context
from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
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.models import CourseEnrollmentAllowed, email_exists_or_retired
from util.password_policy_validators import password_max_length, password_min_length, validate_password
......@@ -47,41 +54,39 @@ class PasswordResetFormNoActive(PasswordResetForm):
raise forms.ValidationError(self.error_messages['unusable'])
return email
def save(
self,
subject_template_name='emails/password_reset_subject.txt',
email_template_name='registration/password_reset_email.html',
use_https=False,
token_generator=default_token_generator,
from_email=configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL),
request=None
):
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 resetting password and sends to the
user.
"""
# This import is here because we are copying and modifying the .save from Django 1.4.5's
# django.contrib.auth.forms.PasswordResetForm directly, which has this import in this place.
from django.core.mail import send_mail
for user in self.users_cache:
site_name = configuration_helpers.get_value(
'SITE_NAME',
settings.SITE_NAME
site = Site.objects.get_current()
message_context = get_base_template_context(site)
message_context.update({
'request': request, # Used by google_analytics_tracking_pixel
# TODO: This overrides `platform_name` from `get_base_template_context` to make the tests passes
'platform_name': configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME),
'reset_link': '{protocol}://{site}{link}'.format(
protocol='https' if use_https else 'http',
site=configuration_helpers.get_value('SITE_NAME', settings.SITE_NAME),
link=reverse('password_reset_confirm', kwargs={
'uidb36': int_to_base36(user.id),
'token': token_generator.make_token(user),
}),
)
})
msg = PasswordReset().personalize(
recipient=Recipient(user.username, user.email),
language=get_user_preference(user, LANGUAGE_KEY),
user_context=message_context,
)
context = {
'email': user.email,
'site_name': site_name,
'uid': int_to_base36(user.id),
'user': user,
'token': token_generator.make_token(user),
'protocol': 'https' if use_https else 'http',
'platform_name': configuration_helpers.get_value('platform_name', settings.PLATFORM_NAME)
}
subject = loader.render_to_string(subject_template_name, context)
# Email subject *must not* contain newlines
subject = subject.replace('\n', '')
email = loader.render_to_string(email_template_name, context)
send_mail(subject, email, from_email, [user.email])
ace.send(msg)
class TrueCheckbox(widgets.CheckboxInput):
......
"""
ACE message types for the student module.
"""
from django.conf import settings
from edx_ace.message import MessageType
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
class PasswordReset(MessageType):
def __init__(self, *args, **kwargs):
super(PasswordReset, self).__init__(*args, **kwargs)
self.options['transactional'] = True
self.options['from_address'] = configuration_helpers.get_value(
'email_from_address', settings.DEFAULT_FROM_EMAIL
)
......@@ -15,6 +15,7 @@ FAKE_SITE = {
"university": "fakeuniversity",
"course_org_filter": "fakeorg",
"platform_name": "Fake University",
"PLATFORM_NAME": "Fake University",
"email_from_address": "no-reply@fakeuniversity.com",
"REGISTRATION_EXTRA_FIELDS": {
"address1": "required",
......
......@@ -11,6 +11,7 @@ from django.contrib.auth.hashers import UNUSABLE_PASSWORD_PREFIX
from django.contrib.auth.models import User
from django.contrib.auth.tokens import default_token_generator
from django.core.cache import cache
from django.core import mail
from django.core.urlresolvers import reverse
from django.test.client import RequestFactory
from django.test.utils import override_settings
......@@ -118,15 +119,12 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
cache.clear()
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', "Test only valid in LMS")
@patch('django.core.mail.send_mail')
@patch('student.views.management.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True))
def test_reset_password_email(self, send_email):
"""
Tests contents of reset password email, and that user is not active
"""
@ddt.data('plain_text', 'html')
def test_reset_password_email(self, body_type):
"""Tests contents of reset password email, and that user is not active"""
good_req = self.request_factory.post('/password_reset/', {'email': self.user.email})
good_req.user = self.user
good_req.site = Mock(domain='example.com')
dop_client = ClientFactory()
dop_access_token = AccessTokenFactory(user=self.user, client=dop_client)
RefreshTokenFactory(user=self.user, client=dop_client, access_token=dop_access_token)
......@@ -140,26 +138,35 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
self.assertFalse(dot_models.AccessToken.objects.filter(user=self.user).exists())
self.assertFalse(dot_models.RefreshToken.objects.filter(user=self.user).exists())
obj = json.loads(good_resp.content)
self.assertEquals(obj, {
'success': True,
'value': "('registration/password_reset_done.html', [])",
})
self.assertTrue(obj['success'])
self.assertIn('e-mailed you instructions for setting your password', obj['value'])
(subject, msg, from_addr, to_addrs) = send_email.call_args[0]
self.assertIn("Password reset", subject)
self.assertIn("You're receiving this e-mail because you requested a password reset", msg)
self.assertEquals(from_addr, configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL))
self.assertEquals(len(to_addrs), 1)
self.assertIn(self.user.email, to_addrs)
from_email = configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL)
sent_message = mail.outbox[0]
bodies = {
'plain_text': sent_message.body,
'html': sent_message.alternatives[0][0],
}
body = bodies[body_type]
self.assertIn("Password reset", sent_message.subject)
self.assertIn("You're receiving this e-mail because you requested a password reset", body)
self.assertEquals(sent_message.from_email, from_email)
self.assertEquals(len(sent_message.to), 1)
self.assertIn(self.user.email, sent_message.to)
self.assert_event_emitted(
SETTING_CHANGE_INITIATED, user_id=self.user.id, setting=u'password', old=None, new=None,
)
#test that the user is not active
# Test that the user is not active
self.user = User.objects.get(pk=self.user.pk)
self.assertFalse(self.user.is_active)
re.search(r'password_reset_confirm/(?P<uidb36>[0-9A-Za-z]+)-(?P<token>.+)/', msg).groupdict()
self.assertIn('password_reset_confirm/', body)
re.search(r'password_reset_confirm/(?P<uidb36>[0-9A-Za-z]+)-(?P<token>.+)/', body).groupdict()
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', "Test only valid in LMS")
@patch('django.core.mail.send_mail')
......@@ -172,6 +179,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
req = self.request_factory.post(
'/password_reset/', {'email': self.user.email}
)
req.site = Mock(domain='example.com')
req.is_secure = Mock(return_value=is_secure)
req.user = self.user
password_reset(req)
......@@ -199,6 +207,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
'/password_reset/', {'email': self.user.email}
)
req.user = self.user
req.site = Mock(domain='example.com')
password_reset(req)
_, msg, _, _ = send_email.call_args[0]
......@@ -216,8 +225,8 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', "Test only valid in LMS")
@patch("openedx.core.djangoapps.site_configuration.helpers.get_value", fake_get_value)
@patch('django.core.mail.send_mail')
def test_reset_password_email_configuration_override(self, send_email):
@ddt.data('plain_text', 'html')
def test_reset_password_email_configuration_override(self, body_type):
"""
Tests that the right url domain and platform name is included in
the reset password email
......@@ -226,18 +235,28 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
'/password_reset/', {'email': self.user.email}
)
req.get_host = Mock(return_value=None)
req.site = Mock(domain='example.com')
req.user = self.user
password_reset(req)
_, msg, from_addr, _ = send_email.call_args[0]
reset_msg = "you requested a password reset for your user account at {}".format(fake_get_value('platform_name'))
with patch('crum.get_current_request', return_value=req):
password_reset(req)
sent_message = mail.outbox[0]
bodies = {
'plain_text': sent_message.body,
'html': sent_message.alternatives[0][0],
}
body = bodies[body_type]
reset_msg = "you requested a password reset for your user account at {}".format(fake_get_value('PLATFORM_NAME'))
self.assertIn(reset_msg, msg)
self.assertIn(reset_msg, body)
self.assert_event_emitted(
SETTING_CHANGE_INITIATED, user_id=self.user.id, setting=u'password', old=None, new=None
)
self.assertEqual(from_addr, "no-reply@fakeuniversity.com")
self.assertEqual(sent_message.from_email, "no-reply@fakeuniversity.com")
@ddt.data(
('invalidUid', 'invalid_token'),
......@@ -391,6 +410,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
'/password_reset/', {'email': self.user.email}
)
req.user = self.user
req.site = Mock(domain='example.com')
password_reset(req)
subj, _, _, _ = send_email.call_args[0]
......
{% 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 "Password Reset" %}
</h1>
<p style="color: rgba(0,0,0,.75);">
{% blocktrans %}You're receiving this e-mail because you requested a password reset for your user account at {{ platform_name }}.{% endblocktrans %}
<br />
</p>
{% if failed %}
<p style="color: rgba(0,0,0,.75);">
{% blocktrans %}However, there is currently no user account associated with your email address: {{ email_address }}.{% endblocktrans %}
<br />
</p>
<p style="color: rgba(0,0,0,.75);">
{% trans "If you did not request this change, you can ignore this email." %}
<br />
</p>
{% else %}
<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 reset your password." %}
<br />
</p>
{% trans "Change 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 %}
{% endif %}
</td>
</tr>
</table>
{% endblock %}
......@@ -7,15 +7,12 @@
{% trans "If you did not request this change, you can ignore this email." %}
{% else %}
{% trans "Please go to the following page and choose a new password:" %}
{% block reset_link %}
{{ protocol }}://{{ site_name }}{% url 'password_reset_confirm' uidb36=uid token=token %}
{% endblock %}
{{ reset_link }}
{% trans "If you didn't request this change, you can disregard this email - we have not yet reset your password." %}
{% trans "Thanks for using our site!" %}
{% endif %}
{% blocktrans %}The {{ platform_name }} Team{% endblocktrans %}
{% endautoescape %}
{{ platform_name }}
{% extends 'ace_common/edx_ace/common/base_head.html' %}
{% load i18n %}
{% autoescape off %}
{% blocktrans %}Password reset on {{ platform_name }}{% endblocktrans %}
{% blocktrans trimmed %}Password reset on {{ platform_name }}{% endblocktrans %}
{% endautoescape %}
......@@ -164,13 +164,18 @@ class StudentAccountUpdateTest(CacheIsolationTestCase, UrlResetMixin):
self.assertEqual(len(mail.outbox), 1)
# Verify that the body contains the failed password reset message
email_body = mail.outbox[0].body
self.assertIn(
'However, there is currently no user account associated with your email address: {email}'.format(
sent_message = mail.outbox[0]
text_body = sent_message.body
html_body = sent_message.alternatives[0][0]
for email_body in [text_body, html_body]:
msg = 'However, there is currently no user account associated with your email address: {email}'.format(
email=bad_email
),
email_body,
)
)
assert u'reset for your user account at {}'.format(settings.PLATFORM_NAME) in email_body
assert 'password_reset_confirm' not in email_body, 'The link should not be added if user was not found'
assert msg in email_body
@ddt.data(True, False)
def test_password_change_logged_out(self, send_email):
......
......@@ -9,19 +9,22 @@ from django.conf import settings
from django.contrib import messages
from django.contrib.auth import get_user_model
from django.contrib.auth.decorators import login_required
from django.core.mail import send_mail
from django.core.urlresolvers import resolve, reverse
from django.contrib.sites.models import Site
from django.core.urlresolvers import reverse
from django.http import HttpRequest, HttpResponse, HttpResponseBadRequest, HttpResponseForbidden
from django.shortcuts import redirect
from django.template import loader
from django.utils.translation import ugettext as _
from django.views.decorators.csrf import ensure_csrf_cookie
from django.views.decorators.http import require_http_methods
from django_countries import countries
import third_party_auth
from edx_ace import ace
from edx_ace.recipient import Recipient
from edxmako.shortcuts import render_to_response
from lms.djangoapps.commerce.models import CommerceConfiguration
from lms.djangoapps.commerce.utils import EcommerceService
from openedx.core.djangoapps.ace_common.template_context import get_base_template_context
from openedx.core.djangoapps.commerce.utils import ecommerce_api_client
from openedx.core.djangoapps.external_auth.login_and_register import login as external_auth_login
from openedx.core.djangoapps.external_auth.login_and_register import register as external_auth_register
......@@ -48,6 +51,7 @@ from openedx.features.enterprise_support.utils import (
update_account_settings_context_for_enterprise,
)
from student.helpers import destroy_oauth_tokens, get_next_url_for_login_page
from student.message_types import PasswordReset
from student.models import UserProfile
from student.views import register_user as old_register_view, signin_user as old_login_view
from third_party_auth import pipeline
......@@ -55,6 +59,7 @@ from third_party_auth.decorators import xframe_allow_whitelisted
from util.bad_request_rate_limiter import BadRequestRateLimiter
from util.date_utils import strftime_localized
AUDIT_LOG = logging.getLogger("audit")
log = logging.getLogger(__name__)
User = get_user_model() # pylint:disable=invalid-name
......@@ -222,20 +227,23 @@ def password_change_request_handler(request):
# no user associated with the email
if configuration_helpers.get_value('ENABLE_PASSWORD_RESET_FAILURE_EMAIL',
settings.FEATURES['ENABLE_PASSWORD_RESET_FAILURE_EMAIL']):
context = {
site = Site.objects.get_current()
message_context = get_base_template_context(site)
message_context.update({
'failed': True,
'request': request, # Used by google_analytics_tracking_pixel
'email_address': email,
'platform_name': configuration_helpers.get_value('platform_name', settings.PLATFORM_NAME),
}
subject = loader.render_to_string('emails/password_reset_subject.txt', context)
subject = ''.join(subject.splitlines())
message = loader.render_to_string('registration/password_reset_email.html', context)
from_email = configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL)
try:
send_mail(subject, message, from_email, [email])
except Exception: # pylint: disable=broad-except
log.exception(u'Unable to send password reset failure email notification from "%s"', from_email)
})
msg = PasswordReset().personalize(
recipient=Recipient(username='', email_address=email),
language=settings.LANGUAGE_CODE,
user_context=message_context,
)
ace.send(msg)
except UserAPIInternalError as err:
log.exception('Error occured during password change for user {email}: {error}'
.format(email=email, error=err))
......
......@@ -39,7 +39,8 @@ for log_name, log_level in LOG_OVERRIDES:
################################ EMAIL ########################################
EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend'
EMAIL_BACKEND = 'django.core.mail.backends.filebased.EmailBackend'
EMAIL_FILE_PATH = '/edx/src/ace_messages/'
############################ PYFS XBLOCKS SERVICE #############################
# Set configuration for Django pyfilesystem
......
......@@ -19,7 +19,7 @@ class AceCommonConfig(AppConfig):
ProjectType.LMS: {
SettingsType.AWS: {PluginSettings.RELATIVE_PATH: u'settings.aws'},
SettingsType.COMMON: {PluginSettings.RELATIVE_PATH: u'settings.common'},
SettingsType.DEVSTACK: {PluginSettings.RELATIVE_PATH: u'settings.common'},
SettingsType.DEVSTACK: {PluginSettings.RELATIVE_PATH: u'settings.devstack'},
}
}
}
......@@ -14,3 +14,10 @@ def plugin_settings(settings):
'ACE_CHANNEL_SAILTHRU_API_SECRET', settings.ACE_CHANNEL_SAILTHRU_API_SECRET,
)
settings.ACE_ROUTING_KEY = settings.ENV_TOKENS.get('ACE_ROUTING_KEY', settings.ACE_ROUTING_KEY)
settings.ACE_CHANNEL_DEFAULT_EMAIL = settings.ENV_TOKENS.get(
'ACE_CHANNEL_DEFAULT_EMAIL', settings.ACE_CHANNEL_DEFAULT_EMAIL
)
settings.ACE_CHANNEL_TRANSACTIONAL_EMAIL = settings.ENV_TOKENS.get(
'ACE_CHANNEL_TRANSACTIONAL_EMAIL', settings.ACE_CHANNEL_TRANSACTIONAL_EMAIL
)
def plugin_settings(settings):
settings.ACE_ENABLED_CHANNELS = [
'file_email'
'django_email'
]
settings.ACE_ENABLED_POLICIES = [
'bulk_email_optout'
......@@ -9,6 +9,8 @@ def plugin_settings(settings):
settings.ACE_CHANNEL_SAILTHRU_TEMPLATE_NAME = 'Automated Communication Engine Email'
settings.ACE_CHANNEL_SAILTHRU_API_KEY = None
settings.ACE_CHANNEL_SAILTHRU_API_SECRET = None
settings.ACE_CHANNEL_DEFAULT_EMAIL = 'django_email'
settings.ACE_CHANNEL_TRANSACTIONAL_EMAIL = 'django_email'
settings.ACE_ROUTING_KEY = 'edx.core.low'
......
"""
Settings for edX ACE on devstack.
"""
from openedx.core.djangoapps.ace_common.settings import common
def plugin_settings(settings):
"""
Override common settings and use `file_email` for better debugging.
"""
common.plugin_settings(settings)
settings.ACE_ENABLED_CHANNELS = [
'file_email'
]
settings.ACE_CHANNEL_DEFAULT_EMAIL = 'file_email'
settings.ACE_CHANNEL_TRANSACTIONAL_EMAIL = 'file_email'
......@@ -20,9 +20,9 @@
{% block preview_text %}{% endblock %}
</div>
{# Note {beacon_src} is not a template variable that is evaluated by the Django template engine. It is evaluated by #}
{# Sailthru when the email is sent. Other email providers would need to replace this variable in the HTML as well. #}
<img src="{beacon_src}" alt="" role="presentation" aria-hidden="true" />
{% for image_src in channel.tracker_image_sources %}
<img src="{image_src}" alt="" role="presentation" aria-hidden="true" />
{% endfor %}
{% google_analytics_tracking_pixel %}
......@@ -157,17 +157,13 @@
<tr>
<!-- Actions -->
<td style="padding-bottom: 20px;">
{# Note that these variables are evaluated by Sailthru, not the Django template engine #}
<p>
<a href="{view_url}" style="color: #005686">
<font color="#005686"><b>{% trans "View on Web" %}</b></font>
</a>
</p>
<p>
<a href="{optout_confirm_url}" style="color: #005686">
<font color="#005686"><b>{% trans "Unsubscribe from this list" %}</b></font>
</a>
</p>
{% for action_link_url, action_link_text in channel.action_links %}
<p>
<a href="{{ action_link_url }}" style="color: #005686">
<font color="#005686"><b>{{ action_link_text }}</b></font>
</a>
</p>
{% endfor %}
</td>
</tr>
<tr>
......
......@@ -7,8 +7,8 @@ import attr
import ddt
import pytz
from django.conf import settings
from edx_ace.channel import ChannelType
from edx_ace.test_utils import StubPolicy, patch_channels, patch_policies
from edx_ace.channel import ChannelMap, ChannelType
from edx_ace.test_utils import StubPolicy, patch_policies
from edx_ace.utils.date import serialize
from freezegun import freeze_time
from mock import Mock, patch
......@@ -383,11 +383,16 @@ class ScheduleSendEmailTestMixin(FilteredQueryCountMixin):
)
patch_policies(self, [StubPolicy([ChannelType.PUSH])])
mock_channel = Mock(
name='test_channel',
channel_type=ChannelType.EMAIL
channel_type=ChannelType.EMAIL,
action_links=[],
tracker_image_sources=[],
)
patch_channels(self, [mock_channel])
channel_map = ChannelMap([
['sailthru', mock_channel],
])
sent_messages = []
with self.settings(TEMPLATES=self._get_template_overrides()):
......@@ -411,8 +416,9 @@ class ScheduleSendEmailTestMixin(FilteredQueryCountMixin):
with self.assertNumQueries(NUM_QUERIES_PER_MESSAGE_DELIVERY):
with patch('analytics.track') as mock_analytics_track:
self.deliver_task(*sent_messages[0])
self.assertEqual(mock_analytics_track.call_count, 1)
with patch('edx_ace.channel.channels', return_value=channel_map):
self.deliver_task(*sent_messages[0])
self.assertEqual(mock_analytics_track.call_count, 1)
self.assertEqual(mock_channel.deliver.call_count, 1)
for (_name, (_msg, email), _kwargs) in mock_channel.deliver.mock_calls:
......
......@@ -11,6 +11,7 @@ from django.core.exceptions import ObjectDoesNotExist
from django.conf import settings
from django.core.validators import validate_email, ValidationError
from django.http import HttpResponseForbidden
from openedx.core.djangoapps.theming.helpers import get_current_request
from six import text_type
from student.models import User, UserProfile, Registration, email_exists_or_retired
......@@ -432,7 +433,8 @@ def request_password_change(email, is_secure):
# 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
use_https=is_secure,
request=get_current_request(),
)
else:
# No user with the provided email address exists.
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment