diff --git a/openedx/core/djangoapps/user_authn/message_types.py b/openedx/core/djangoapps/user_authn/message_types.py index b0043578ec..ab14ba12b5 100644 --- a/openedx/core/djangoapps/user_authn/message_types.py +++ b/openedx/core/djangoapps/user_authn/message_types.py @@ -2,13 +2,25 @@ ACE message types for user_authn-related emails. """ - from openedx.core.djangoapps.ace_common.message import BaseMessageType class PasswordReset(BaseMessageType): + """ + A message to the user with password reset link. + """ def __init__(self, *args, **kwargs): super(PasswordReset, self).__init__(*args, **kwargs) # pylint: disable=unsupported-assignment-operation self.options['transactional'] = True + + +class PasswordResetSuccess(BaseMessageType): + """ + A message to the user when the password rest was successful. + """ + + def __init__(self, *args, **kwargs): + super(PasswordResetSuccess, self).__init__(*args, **kwargs) + self.options['transactional'] = True diff --git a/openedx/core/djangoapps/user_authn/templates/user_authn/edx_ace/passwordresetsuccess/email/body.html b/openedx/core/djangoapps/user_authn/templates/user_authn/edx_ace/passwordresetsuccess/email/body.html new file mode 100644 index 0000000000..7e43b8613e --- /dev/null +++ b/openedx/core/djangoapps/user_authn/templates/user_authn/edx_ace/passwordresetsuccess/email/body.html @@ -0,0 +1,31 @@ +{% extends 'ace_common/edx_ace/common/base_body.html' %} + +{% load i18n %} +{% load static %} +{% block content %} + + + + +
+

+ {% trans "Password Reset Success" as tmsg %}{{ tmsg | force_escape }} +

+

+ {% filter force_escape %} + {% blocktrans %}Hello {{ name }},{% endblocktrans %} + {% endfilter %} +

+

+ {% filter force_escape %} + {% blocktrans %}This is to confirm that you have successfully changed your password associated with {{ platform_name }} account. Please sign-in to your account.{% endblocktrans %} + {% endfilter %} +
+

+ + {% filter force_escape %} + {% blocktrans asvar course_cta_text %}Sign in{% endblocktrans %} + {% endfilter %} + {% include "ace_common/edx_ace/common/return_to_course_cta.html" with course_cta_text=course_cta_text course_cta_url=login_link %} +
+{% endblock %} diff --git a/openedx/core/djangoapps/user_authn/templates/user_authn/edx_ace/passwordresetsuccess/email/body.txt b/openedx/core/djangoapps/user_authn/templates/user_authn/edx_ace/passwordresetsuccess/email/body.txt new file mode 100644 index 0000000000..0fb9e5900f --- /dev/null +++ b/openedx/core/djangoapps/user_authn/templates/user_authn/edx_ace/passwordresetsuccess/email/body.txt @@ -0,0 +1,6 @@ +{% load i18n %}{% autoescape off %} +{% blocktrans %}Hello {{ name }}, {% endblocktrans %} +{% blocktrans %}This is to confirm that you have successfully changed your password associated with {{ platform_name }} account. Please sign-in to your account.{% endblocktrans %} + +{{ login_link }} +{% endautoescape %} diff --git a/openedx/core/djangoapps/user_authn/templates/user_authn/edx_ace/passwordresetsuccess/email/from_name.txt b/openedx/core/djangoapps/user_authn/templates/user_authn/edx_ace/passwordresetsuccess/email/from_name.txt new file mode 100644 index 0000000000..dcbc23c004 --- /dev/null +++ b/openedx/core/djangoapps/user_authn/templates/user_authn/edx_ace/passwordresetsuccess/email/from_name.txt @@ -0,0 +1 @@ +{{ platform_name }} diff --git a/openedx/core/djangoapps/user_authn/templates/user_authn/edx_ace/passwordresetsuccess/email/head.html b/openedx/core/djangoapps/user_authn/templates/user_authn/edx_ace/passwordresetsuccess/email/head.html new file mode 100644 index 0000000000..366ada7ad9 --- /dev/null +++ b/openedx/core/djangoapps/user_authn/templates/user_authn/edx_ace/passwordresetsuccess/email/head.html @@ -0,0 +1 @@ +{% extends 'ace_common/edx_ace/common/base_head.html' %} diff --git a/openedx/core/djangoapps/user_authn/templates/user_authn/edx_ace/passwordresetsuccess/email/subject.txt b/openedx/core/djangoapps/user_authn/templates/user_authn/edx_ace/passwordresetsuccess/email/subject.txt new file mode 100644 index 0000000000..7445534c02 --- /dev/null +++ b/openedx/core/djangoapps/user_authn/templates/user_authn/edx_ace/passwordresetsuccess/email/subject.txt @@ -0,0 +1,4 @@ +{% load i18n %} +{% autoescape off %} +{% blocktrans trimmed %}Password reset completed on {{ platform_name }}{% endblocktrans %} +{% endautoescape %} diff --git a/openedx/core/djangoapps/user_authn/views/password_reset.py b/openedx/core/djangoapps/user_authn/views/password_reset.py index 692ddc0a00..752de48807 100644 --- a/openedx/core/djangoapps/user_authn/views/password_reset.py +++ b/openedx/core/djangoapps/user_authn/views/password_reset.py @@ -38,7 +38,7 @@ from openedx.core.djangoapps.user_api.accounts.utils import is_secondary_email_f from openedx.core.djangoapps.user_api.helpers import FormDescription from openedx.core.djangoapps.user_api.models import UserRetirementRequest from openedx.core.djangoapps.user_api.preferences.api import get_user_preference -from openedx.core.djangoapps.user_authn.message_types import PasswordReset +from openedx.core.djangoapps.user_authn.message_types import PasswordReset, PasswordResetSuccess from openedx.core.djangolib.markup import HTML from student.forms import send_account_recovery_email_for_user from student.models import AccountRecovery @@ -54,6 +54,16 @@ log = logging.getLogger("edx.student") AUDIT_LOG = logging.getLogger("audit") +def get_user_default_email_params(user): + """ + Get default email params for the user. + """ + site = get_current_site() + message_context = get_base_template_context(site) + user_language_pref = get_user_preference(user, LANGUAGE_KEY) + return [message_context, user_language_pref] + + def get_password_reset_form(): """Return a description of the password reset form. @@ -100,6 +110,31 @@ def get_password_reset_form(): return form_desc +def send_password_reset_success_email(user, request): + """ + Send an email to user indicating that password reset was successful. + + Arguments: + user (User): Django User object + request (HttpRequest): Django request object + """ + message_context, user_language_preference = get_user_default_email_params(user) + lms_root_url = configuration_helpers.get_value('LMS_ROOT_URL', settings.LMS_ROOT_URL) + message_context.update( + {'login_link': '{}/login'.format(lms_root_url), 'request': request, } + ) + + msg = PasswordResetSuccess(context=message_context).personalize( + recipient=Recipient(user.username, user.email), + language=user_language_preference, + user_context={"name": user.profile.name}, + ) + try: + ace.send(msg) + except Exception: # pylint: disable=broad-except + log.exception('PasswordResetSuccess: sending email to user [%s] failed.', user.username) + + def send_password_reset_email_for_user(user, request, preferred_email=None): """ Send out a password reset email for the given user. @@ -109,8 +144,7 @@ def send_password_reset_email_for_user(user, request, preferred_email=None): request (HttpRequest): Django request object preferred_email (str): Send email to this address if present, otherwise fallback to user's email address. """ - site = get_current_site() - message_context = get_base_template_context(site) + message_context, user_language_preference = get_user_default_email_params(user) 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 @@ -127,7 +161,7 @@ def send_password_reset_email_for_user(user, request, preferred_email=None): msg = PasswordReset().personalize( recipient=Recipient(user.username, preferred_email or user.email), - language=get_user_preference(user, LANGUAGE_KEY), + language=user_language_preference, user_context=message_context, ) ace.send(msg) @@ -443,6 +477,7 @@ class PasswordResetConfirmWrapper(PasswordResetConfirmView): 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']) + is_account_recovery = 'is_account_recovery' in request.GET password = request.POST['new_password1'] response = self._validate_password(password, request) @@ -455,17 +490,19 @@ class PasswordResetConfirmWrapper(PasswordResetConfirmView): # 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: + password_reset_successful = response.status_code == 302 + if not password_reset_successful: return self._handle_password_reset_failure(response) updated_user = User.objects.get(id=self.uid_int) - if 'is_account_recovery' in request.GET: + if is_account_recovery: self._handle_primary_email_update(updated_user) updated_user.save() - if response.status_code == 302 and 'is_account_recovery' in request.GET: + if password_reset_successful and is_account_recovery: self._handle_password_creation(request, updated_user) + send_password_reset_success_email(updated_user, request) return response def dispatch(self, *args, **kwargs): diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py b/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py index 2e4b3b6d28..e318c704dd 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py @@ -2,7 +2,6 @@ Test the various password reset flows """ - import json import re import unicodedata @@ -85,6 +84,21 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): process_request(request) request.session[INTERNAL_RESET_SESSION_TOKEN] = self.token + @property + def password_reset_confirm_url(self): + """ + Returns Password reset confirm URL + """ + return reverse("password_reset_confirm", kwargs={"uidb36": self.uidb36, "token": self.token}) + + def send_password_reset_request(self): + """ + Sends GET request on password reset url. + """ + request = self.request_factory.get(self.password_reset_confirm_url) + self.setup_request_session_with_token(request) + return request + @patch( 'openedx.core.djangoapps.user_authn.views.password_reset.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True) @@ -170,6 +184,20 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): cache.clear() + def assert_email_sent_successfully(self, expected): + """ + Verify that the password confirm email has been sent to the user. + """ + from_email = configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL) + sent_message = mail.outbox[0] + body = sent_message.body + + self.assertIn(expected['subject'], sent_message.subject) + self.assertIn(expected['body'], body) + self.assertEqual(sent_message.from_email, from_email) + self.assertEqual(len(sent_message.to), 1) + self.assertIn(self.user.email, sent_message.to) + def test_ratelimitted_from_same_ip_with_different_email(self): """ Test that password reset endpoint allow only one request per minute per IP. @@ -405,12 +433,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): Then reset password page renders And inactive user is set to active """ - url = reverse( - "password_reset_confirm", - kwargs={"uidb36": self.uidb36, "token": self.token} - ) - good_reset_req = self.request_factory.get(url) - process_request(good_reset_req) + good_reset_req = self.send_password_reset_request() good_reset_req.user = self.user redirect_response = PasswordResetConfirmWrapper.as_view()(good_reset_req, uidb36=self.uidb36, token=self.token) @@ -435,12 +458,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): Then reset password page renders And inactive user associated with token is set to active """ - url = reverse( - "password_reset_confirm", - kwargs={"uidb36": self.uidb36, "token": self.token} - ) - good_reset_req = self.request_factory.get(url) - process_request(good_reset_req) + good_reset_req = self.send_password_reset_request() good_reset_req.user = AnonymousUser() redirect_response = PasswordResetConfirmWrapper.as_view()(good_reset_req, uidb36=self.uidb36, token=self.token) @@ -459,12 +477,8 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): """ self.assertFalse(self.user.is_active) - url = reverse( - 'password_reset_confirm', - kwargs={'uidb36': self.uidb36, 'token': self.token} - ) request_params = {'new_password1': 'password1', 'new_password2': 'password2'} - confirm_request = self.request_factory.post(url, data=request_params) + confirm_request = self.request_factory.post(self.password_reset_confirm_url, data=request_params) self.setup_request_session_with_token(confirm_request) confirm_request.user = self.user @@ -485,11 +499,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): # Retire the user. UserRetirementRequest.create_retirement_request(self.user) - url = reverse( - 'password_reset_confirm', - kwargs={'uidb36': self.uidb36, 'token': self.token} - ) - reset_req = self.request_factory.get(url) + reset_req = self.request_factory.get(self.password_reset_confirm_url) reset_req.user = self.user resp = PasswordResetConfirmWrapper.as_view()(reset_req, uidb36=self.uidb36, token=self.token) @@ -505,17 +515,13 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): method of NFKC. In this test, the input password is u'p\u212bssword'. It should be normalized to u'p\xc5ssword' """ - url = reverse( - "password_reset_confirm", - kwargs={"uidb36": self.uidb36, "token": self.token} - ) - password = u'p\u212bssword' request_params = {'new_password1': password, 'new_password2': password} - confirm_request = self.request_factory.post(url, data=request_params) + confirm_request = self.request_factory.post(self.password_reset_confirm_url, data=request_params) process_request(confirm_request) confirm_request.session[INTERNAL_RESET_SESSION_TOKEN] = self.token confirm_request.user = self.user + confirm_request.site = Mock(domain='example.com') __ = PasswordResetConfirmWrapper.as_view()(confirm_request, uidb36=self.uidb36, token=self.token) user = User.objects.get(pk=self.user.pk) @@ -523,6 +529,11 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): expected_user_password = make_password(unicodedata.normalize('NFKC', u'p\u212bssword'), salt_val) self.assertEqual(expected_user_password, user.password) + self.assert_email_sent_successfully({ + 'subject': 'Password reset completed', + 'body': 'This is to confirm that you have successfully changed your password' + }) + @override_settings(AUTH_PASSWORD_VALIDATORS=[ create_validator_config('util.password_policy_validators.MinimumLengthValidator', {'min_length': 2}), create_validator_config('util.password_policy_validators.MaximumLengthValidator', {'max_length': 10}) @@ -542,13 +553,8 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): Tests that if we provide password characters less then PASSWORD_MIN_LENGTH, or more than PASSWORD_MAX_LENGTH, password reset will fail with error message. """ - - url = reverse( - 'password_reset_confirm', - kwargs={'uidb36': self.uidb36, 'token': self.token} - ) request_params = {'new_password1': password_dict['password'], 'new_password2': password_dict['password']} - confirm_request = self.request_factory.post(url, data=request_params) + confirm_request = self.request_factory.post(self.password_reset_confirm_url, data=request_params) self.setup_request_session_with_token(confirm_request) confirm_request.user = self.user @@ -563,12 +569,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): """ Tests password reset confirmation page for site configuration override. """ - url = reverse( - "password_reset_confirm", - kwargs={"uidb36": self.uidb36, "token": self.token} - ) - good_reset_req = self.request_factory.get(url) - process_request(good_reset_req) + good_reset_req = self.send_password_reset_request() good_reset_req.user = self.user PasswordResetConfirmWrapper.as_view()(good_reset_req, uidb36=self.uidb36, token=self.token) confirm_kwargs = reset_confirm.call_args[1] @@ -599,11 +600,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): """ Tests that user should not be able to reset password through other user's token """ - reset_url = reverse( - "password_reset_confirm", - kwargs={"uidb36": self.uidb36, "token": self.token} - ) - reset_request = self.request_factory.get(reset_url) + reset_request = self.request_factory.get(self.password_reset_confirm_url) reset_request.user = UserFactory.create() self.assertRaises(Http404, PasswordResetConfirmWrapper.as_view(), reset_request, uidb36=self.uidb36,