Fixed #27518 -- Prevented possibie password reset token leak via HTTP Referer header.

Thanks Florian Apolloner for contributing to this patch and
Collin Anderson, Markus Holtermann, and Tim Graham for review.

8 files changed, 123 insertions, 11 deletions

diff --git a/AUTHORS b/AUTHORS
index c356dce59f..7365ff8788 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -662,6 +662,7 @@ answer newbie questions, and generally made Django that much better:
     Robert Wittams
     Rob Hudson <http://rob.cogit8.org/>
     Robin Munn <http://www.geekforgod.com/>
+    Romain Garrigues <romain.garrigues.cs@gmail.com>
     Ronny Haryanto <http://ronny.haryan.to/>
     Ross Poulton <ross@rossp.org>
     Rozza <ross.lawley@gmail.com>
diff --git a/django/contrib/auth/tokens.py b/django/contrib/auth/tokens.py
index 46b8467476..6cf694cebb 100644
--- a/django/contrib/auth/tokens.py
+++ b/django/contrib/auth/tokens.py
@@ -24,6 +24,8 @@ class PasswordResetTokenGenerator(object):
         """
         Check that a password reset token is correct for a given user.
         """
+        if not (user and token):
+            return False
         # Parse the token
         try:
             ts_b36, hash = token.split("-")
diff --git a/django/contrib/auth/views.py b/django/contrib/auth/views.py
index 8edf02a7d1..be156286c6 100644
--- a/django/contrib/auth/views.py
+++ b/django/contrib/auth/views.py
@@ -425,6 +425,10 @@ class PasswordResetView(PasswordContextMixin, FormView):
         return super(PasswordResetView, self).form_valid(form)
 
 
+INTERNAL_RESET_URL_TOKEN = 'set-password'
+INTERNAL_RESET_SESSION_TOKEN = '_password_reset_token'
+
+
 class PasswordResetDoneView(PasswordContextMixin, TemplateView):
     template_name = 'registration/password_reset_done.html'
     title = _('Password reset sent')
@@ -446,12 +450,26 @@ class PasswordResetConfirmView(PasswordContextMixin, FormView):
         self.validlink = False
         self.user = self.get_user(kwargs['uidb64'])
 
-        if self.user is not None and self.token_generator.check_token(self.user, kwargs['token']):
-            self.validlink = True
-        else:
-            return self.render_to_response(self.get_context_data())
-
-        return super(PasswordResetConfirmView, self).dispatch(*args, **kwargs)
+        if self.user is not None:
+            token = kwargs['token']
+            if token == INTERNAL_RESET_URL_TOKEN:
+                session_token = self.request.session.get(INTERNAL_RESET_SESSION_TOKEN)
+                if self.token_generator.check_token(self.user, session_token):
+                    # If the token is valid, display the password reset form.
+                    self.validlink = True
+                    return super(PasswordResetConfirmView, self).dispatch(*args, **kwargs)
+            else:
+                if self.token_generator.check_token(self.user, token):
+                    # Store the token in the session and redirect to the
+                    # password reset form at a URL without the token. That
+                    # avoids the possibility of leaking the token in the
+                    # HTTP Referer header.
+                    self.request.session[INTERNAL_RESET_SESSION_TOKEN] = token
+                    redirect_url = self.request.path.replace(token, INTERNAL_RESET_URL_TOKEN)
+                    return HttpResponseRedirect(redirect_url)
+
+        # Display the "Password reset unsuccessful" page.
+        return self.render_to_response(self.get_context_data())
 
     def get_user(self, uidb64):
         try:
@@ -471,6 +489,7 @@ class PasswordResetConfirmView(PasswordContextMixin, FormView):
         user = form.save()
         if self.post_reset_login:
             auth_login(self.request, user)
+        del self.request.session[INTERNAL_RESET_SESSION_TOKEN]
         return super(PasswordResetConfirmView, self).form_valid(form)
 
     def get_context_data(self, **kwargs):
diff --git a/docs/releases/1.11.txt b/docs/releases/1.11.txt
index 9bb9a2e903..c8f3e92f21 100644
--- a/docs/releases/1.11.txt
+++ b/docs/releases/1.11.txt
@@ -116,6 +116,14 @@ Minor features
   :class:`~django.contrib.auth.views.PasswordResetConfirmView` allows
   automatically logging in a user after a successful password reset.
 
+* To avoid the possibility of leaking a password reset token via the HTTP
+  Referer header (for example, if the reset page includes a reference to CSS or
+  JavaScript hosted on another domain), the
+  :class:`~django.contrib.auth.views.PasswordResetConfirmView` (but not the
+  deprecated ``password_reset_confirm()`` function-based view) stores the token
+  in a session and redirects to itself to present the password change form to
+  the user without the token in the URL.
+
 * :func:`~django.contrib.auth.update_session_auth_hash` now rotates the session
   key to allow a password change to invalidate stolen session cookies.
 
diff --git a/tests/auth_tests/client.py b/tests/auth_tests/client.py
new file mode 100644
index 0000000000..004101108b
--- /dev/null
+++ b/tests/auth_tests/client.py
@@ -0,0 +1,41 @@
+import re
+
+from django.contrib.auth.views import (
+    INTERNAL_RESET_SESSION_TOKEN, INTERNAL_RESET_URL_TOKEN,
+)
+from django.test import Client
+
+
+def extract_token_from_url(url):
+    token_search = re.search(r'/reset/.*/(.+?)/', url)
+    if token_search:
+        return token_search.group(1)
+
+
+class PasswordResetConfirmClient(Client):
+    """
+    This client eases testing the password reset flow by emulating the
+    PasswordResetConfirmView's redirect and saving of the reset token in the
+    user's session. This request puts 'my-token' in the session and redirects
+    to '/reset/bla/set-password/':
+
+    >>> client = PasswordResetConfirmClient()
+    >>> client.get('/reset/bla/my-token/')
+    """
+    def _get_password_reset_confirm_redirect_url(self, url):
+        token = extract_token_from_url(url)
+        if not token:
+            return url
+        # Add the token to the session
+        session = self.session
+        session[INTERNAL_RESET_SESSION_TOKEN] = token
+        session.save()
+        return url.replace(token, INTERNAL_RESET_URL_TOKEN)
+
+    def get(self, path, *args, **kwargs):
+        redirect_url = self._get_password_reset_confirm_redirect_url(path)
+        return super(PasswordResetConfirmClient, self).get(redirect_url, *args, **kwargs)
+
+    def post(self, path, *args, **kwargs):
+        redirect_url = self._get_password_reset_confirm_redirect_url(path)
+        return super(PasswordResetConfirmClient, self).post(redirect_url, *args, **kwargs)
diff --git a/tests/auth_tests/test_templates.py b/tests/auth_tests/test_templates.py
index 9414f8b299..a1d14c9774 100644
--- a/tests/auth_tests/test_templates.py
+++ b/tests/auth_tests/test_templates.py
@@ -3,12 +3,15 @@ from django.contrib.auth.models import User
 from django.contrib.auth.tokens import PasswordResetTokenGenerator
 from django.contrib.auth.views import (
     PasswordChangeDoneView, PasswordChangeView, PasswordResetCompleteView,
-    PasswordResetConfirmView, PasswordResetDoneView, PasswordResetView,
+    PasswordResetDoneView, PasswordResetView,
 )
 from django.test import RequestFactory, TestCase, override_settings
+from django.urls import reverse
 from django.utils.encoding import force_bytes, force_text
 from django.utils.http import urlsafe_base64_encode
 
+from .client import PasswordResetConfirmClient
+
 
 @override_settings(ROOT_URLCONF='auth_tests.urls')
 class AuthTemplateTests(TestCase):
@@ -34,16 +37,20 @@ class AuthTemplateTests(TestCase):
 
     def test_PasswordResetConfirmView_invalid_token(self):
         # PasswordResetConfirmView invalid token
-        response = PasswordResetConfirmView.as_view(success_url='dummy/')(self.request, uidb64='Bad', token='Bad')
+        client = PasswordResetConfirmClient()
+        url = reverse('password_reset_confirm', kwargs={'uidb64': 'Bad', 'token': 'Bad-Token'})
+        response = client.get(url)
         self.assertContains(response, '<title>Password reset unsuccessful</title>')
         self.assertContains(response, '<h1>Password reset unsuccessful</h1>')
 
     def test_PasswordResetConfirmView_valid_token(self):
         # PasswordResetConfirmView valid token
+        client = PasswordResetConfirmClient()
         default_token_generator = PasswordResetTokenGenerator()
         token = default_token_generator.make_token(self.user)
         uidb64 = force_text(urlsafe_base64_encode(force_bytes(self.user.pk)))
-        response = PasswordResetConfirmView.as_view(success_url='dummy/')(self.request, uidb64=uidb64, token=token)
+        url = reverse('password_reset_confirm', kwargs={'uidb64': uidb64, 'token': token})
+        response = client.get(url)
         self.assertContains(response, '<title>Enter new password</title>')
         self.assertContains(response, '<h1>Enter new password</h1>')
 
diff --git a/tests/auth_tests/test_tokens.py b/tests/auth_tests/test_tokens.py
index 99f9741a0a..7ff3f15f3d 100644
--- a/tests/auth_tests/test_tokens.py
+++ b/tests/auth_tests/test_tokens.py
@@ -62,3 +62,10 @@ class TokenGeneratorTest(TestCase):
         # This will put a 14-digit base36 timestamp into the token, which is too large.
         with self.assertRaises(ValueError):
             p0._make_token_with_timestamp(user, 175455491841851871349)
+
+    def test_check_token_with_nonexistent_token_and_user(self):
+        user = User.objects.create_user('tokentestuser', 'test2@example.com', 'testpw')
+        p0 = PasswordResetTokenGenerator()
+        tk1 = p0.make_token(user)
+        self.assertIs(p0.check_token(None, tk1), False)
+        self.assertIs(p0.check_token(user, None), False)
diff --git a/tests/auth_tests/test_views.py b/tests/auth_tests/test_views.py
index 209f9f698a..2d0d2ae96f 100644
--- a/tests/auth_tests/test_views.py
+++ b/tests/auth_tests/test_views.py
@@ -16,7 +16,8 @@ from django.contrib.auth.forms import (
 )
 from django.contrib.auth.models import User
 from django.contrib.auth.views import (
-    LoginView, logout_then_login, redirect_to_login,
+    INTERNAL_RESET_SESSION_TOKEN, LoginView, logout_then_login,
+    redirect_to_login,
 )
 from django.contrib.sessions.middleware import SessionMiddleware
 from django.contrib.sites.requests import RequestSite
@@ -24,7 +25,7 @@ from django.core import mail
 from django.db import connection
 from django.http import HttpRequest, QueryDict
 from django.middleware.csrf import CsrfViewMiddleware, get_token
-from django.test import TestCase, override_settings
+from django.test import Client, TestCase, override_settings
 from django.test.utils import patch_logger
 from django.urls import NoReverseMatch, reverse, reverse_lazy
 from django.utils.deprecation import RemovedInDjango21Warning
@@ -33,6 +34,7 @@ from django.utils.http import urlquote
 from django.utils.six.moves.urllib.parse import ParseResult, urlparse
 from django.utils.translation import LANGUAGE_SESSION_KEY
 
+from .client import PasswordResetConfirmClient
 from .models import CustomUser, UUIDUser
 from .settings import AUTH_TEMPLATES
 
@@ -116,6 +118,9 @@ class AuthViewNamedURLTests(AuthViewsTestCase):
 
 class PasswordResetTest(AuthViewsTestCase):
 
+    def setUp(self):
+        self.client = PasswordResetConfirmClient()
+
     def test_email_not_found(self):
         """If the provided email is not registered, don't raise any error but
         also don't send any email."""
@@ -278,6 +283,8 @@ class PasswordResetTest(AuthViewsTestCase):
         # Check the password has been changed
         u = User.objects.get(email='staffmember@example.com')
         self.assertTrue(u.check_password("anewpassword"))
+        # The reset token is deleted from the session.
+        self.assertNotIn(INTERNAL_RESET_SESSION_TOKEN, self.client.session)
 
         # Check we can't use the link again
         response = self.client.get(path)
@@ -338,6 +345,23 @@ class PasswordResetTest(AuthViewsTestCase):
         response = self.client.get('/reset/zzzzzzzzzzzzz/1-1/')
         self.assertContains(response, "Hello, .")
 
+    def test_confirm_link_redirects_to_set_password_page(self):
+        url, path = self._test_confirm_start()
+        # Don't use PasswordResetConfirmClient (self.client) here which
+        # automatically fetches the redirect page.
+        client = Client()
+        response = client.get(path)
+        token = response.resolver_match.kwargs['token']
+        uuidb64 = response.resolver_match.kwargs['uidb64']
+        self.assertRedirects(response, '/reset/%s/set-password/' % uuidb64)
+        self.assertEqual(client.session['_password_reset_token'], token)
+
+    def test_invalid_link_if_going_directly_to_the_final_reset_password_url(self):
+        url, path = self._test_confirm_start()
+        _, uuidb64, _ = path.strip('/').split('/')
+        response = Client().get('/reset/%s/set-password/' % uuidb64)
+        self.assertContains(response, 'The password reset link was invalid')
+
 
 @override_settings(AUTH_USER_MODEL='auth_tests.CustomUser')
 class CustomUserPasswordResetTest(AuthViewsTestCase):
@@ -352,6 +376,9 @@ class CustomUserPasswordResetTest(AuthViewsTestCase):
         cls.u1.set_password('password')
         cls.u1.save()
 
+    def setUp(self):
+        self.client = PasswordResetConfirmClient()
+
     def _test_confirm_start(self):
         # Start by creating the email
         response = self.client.post('/password_reset/', {'email': self.user_email})