Merge pull request #15780 from edx/nedbat/learner-2172-ficus

Host poisoning vulnerability fix

common/djangoapps/student/forms.py

@@ -48,7 +48,6 @@ class PasswordResetFormNoActive(PasswordResetForm):
 
     def save(
             self,
-            domain_override=None,
             subject_template_name='registration/password_reset_subject.txt',
             email_template_name='registration/password_reset_email.html',
             use_https=False,
@@ -64,13 +63,10 @@ class PasswordResetFormNoActive(PasswordResetForm):
         # 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:
-            if not domain_override:
-                site_name = configuration_helpers.get_value(
-                    'SITE_NAME',
-                    settings.SITE_NAME
-                )
-            else:
-                site_name = domain_override
+            site_name = configuration_helpers.get_value(
+                'SITE_NAME',
+                settings.SITE_NAME
+            )
             context = {
                 'email': user.email,
                 'site_name': site_name,

common/djangoapps/student/tests/test_reset_password.py

@@ -176,36 +176,33 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
 
     @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', "Test only valid in LMS")
     @patch('django.core.mail.send_mail')
-    @ddt.data(('Crazy Awesome Site', 'Crazy Awesome Site'), (None, 'edX'))
+    @ddt.data(('Crazy Awesome Site', 'Crazy Awesome Site'), ('edX', 'edX'))
     @ddt.unpack
-    def test_reset_password_email_domain(self, domain_override, platform_name, send_email):
+    def test_reset_password_email_site(self, site_name, platform_name, send_email):
         """
         Tests that the right url domain and platform name is included in
         the reset password email
         """
         with patch("django.conf.settings.PLATFORM_NAME", platform_name):
-            req = self.request_factory.post(
-                '/password_reset/', {'email': self.user.email}
-            )
-            req.get_host = Mock(return_value=domain_override)
-            req.user = self.user
-            password_reset(req)
-            _, msg, _, _ = send_email.call_args[0]
+            with patch("django.conf.settings.SITE_NAME", site_name):
+                req = self.request_factory.post(
+                    '/password_reset/', {'email': self.user.email}
+                )
+                req.user = self.user
+                password_reset(req)
+                _, msg, _, _ = send_email.call_args[0]
 
-            reset_msg = "you requested a password reset for your user account at {}"
-            if domain_override:
-                reset_msg = reset_msg.format(domain_override)
-            else:
-                reset_msg = reset_msg.format(settings.SITE_NAME)
+                reset_msg = "you requested a password reset for your user account at {}"
+                reset_msg = reset_msg.format(site_name)
 
-            self.assertIn(reset_msg, msg)
+                self.assertIn(reset_msg, msg)
 
-            sign_off = "The {} Team".format(platform_name)
-            self.assertIn(sign_off, msg)
+                sign_off = "The {} Team".format(platform_name)
+                self.assertIn(sign_off, msg)
 
-            self.assert_event_emitted(
-                SETTING_CHANGE_INITIATED, user_id=self.user.id, setting=u'password', old=None, new=None
-            )
+                self.assert_event_emitted(
+                    SETTING_CHANGE_INITIATED, user_id=self.user.id, setting=u'password', old=None, new=None
+                )
 
     @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', "Test only valid in LMS")
     @patch("openedx.core.djangoapps.site_configuration.helpers.get_value", fake_get_value)

common/djangoapps/student/views.py

@@ -2196,8 +2196,7 @@ def password_reset(request):
     if form.is_valid():
         form.save(use_https=request.is_secure(),
                   from_email=configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL),
-                  request=request,
-                  domain_override=request.get_host())
+                  request=request)
         # When password change is complete, a "edx.user.settings.changed" event will be emitted.
         # But because changing the password is multi-step, we also emit an event here so that we can
         # track where the request was initiated.

lms/djangoapps/student_account/views.py

@@ -175,7 +175,7 @@ def password_change_request_handler(request):
 
     if email:
         try:
-            request_password_change(email, request.get_host(), request.is_secure())
+            request_password_change(email, request.is_secure())
             user = user if user.is_authenticated() else User.objects.get(email=email)
             destroy_oauth_tokens(user)
         except UserNotFound:

openedx/core/djangoapps/user_api/accounts/api.py

@@ -371,7 +371,7 @@ def activate_account(activation_key):
 
 
 @intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError])
-def request_password_change(email, orig_host, is_secure):
+def request_password_change(email, is_secure):
     """Email a single-use link for performing a password reset.
 
     Users must confirm the password change before we update their information.
@@ -399,7 +399,6 @@ def request_password_change(email, orig_host, is_secure):
         # and email it to the user.
         form.save(
             from_email=configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL),
-            domain_override=orig_host,
             use_https=is_secure
         )
     else:

openedx/core/djangoapps/user_api/accounts/tests/test_api.py

@@ -296,7 +296,6 @@ class AccountCreationActivationAndPasswordChangeTest(TestCase):
     PASSWORD = u'ṕáśśẃőŕd'
     EMAIL = u'frank+underwood@example.com'
 
-    ORIG_HOST = 'example.com'
     IS_SECURE = False
 
     INVALID_USERNAMES = [
@@ -400,7 +399,7 @@ class AccountCreationActivationAndPasswordChangeTest(TestCase):
         activate_account(activation_key)
 
         # Request a password change
-        request_password_change(self.EMAIL, self.ORIG_HOST, self.IS_SECURE)
+        request_password_change(self.EMAIL, self.IS_SECURE)
 
         # Verify that one email message has been sent
         self.assertEqual(len(mail.outbox), 1)
@@ -414,7 +413,7 @@ class AccountCreationActivationAndPasswordChangeTest(TestCase):
     @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in LMS')
     def test_request_password_change_invalid_user(self):
         with self.assertRaises(UserNotFound):
-            request_password_change(self.EMAIL, self.ORIG_HOST, self.IS_SECURE)
+            request_password_change(self.EMAIL, self.IS_SECURE)
 
         # Verify that no email messages have been sent
         self.assertEqual(len(mail.outbox), 0)
@@ -424,7 +423,7 @@ class AccountCreationActivationAndPasswordChangeTest(TestCase):
         # Create an account, but do not activate it
         create_account(self.USERNAME, self.PASSWORD, self.EMAIL)
 
-        request_password_change(self.EMAIL, self.ORIG_HOST, self.IS_SECURE)
+        request_password_change(self.EMAIL, self.IS_SECURE)
 
         # Verify that the activation email was still sent
         self.assertEqual(len(mail.outbox), 1)