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.
This commit is contained in:
Romain Garrigues 2017-01-13 14:17:54 +00:00 committed by Tim Graham
parent 91023d79ec
commit ede59ef6f3
8 changed files with 122 additions and 10 deletions

View File

@ -662,6 +662,7 @@ answer newbie questions, and generally made Django that much better:
Robert Wittams Robert Wittams
Rob Hudson <http://rob.cogit8.org/> Rob Hudson <http://rob.cogit8.org/>
Robin Munn <http://www.geekforgod.com/> Robin Munn <http://www.geekforgod.com/>
Romain Garrigues <romain.garrigues.cs@gmail.com>
Ronny Haryanto <http://ronny.haryan.to/> Ronny Haryanto <http://ronny.haryan.to/>
Ross Poulton <ross@rossp.org> Ross Poulton <ross@rossp.org>
Rozza <ross.lawley@gmail.com> Rozza <ross.lawley@gmail.com>

View File

@ -24,6 +24,8 @@ class PasswordResetTokenGenerator(object):
""" """
Check that a password reset token is correct for a given user. Check that a password reset token is correct for a given user.
""" """
if not (user and token):
return False
# Parse the token # Parse the token
try: try:
ts_b36, hash = token.split("-") ts_b36, hash = token.split("-")

View File

@ -425,6 +425,10 @@ class PasswordResetView(PasswordContextMixin, FormView):
return super(PasswordResetView, self).form_valid(form) return super(PasswordResetView, self).form_valid(form)
INTERNAL_RESET_URL_TOKEN = 'set-password'
INTERNAL_RESET_SESSION_TOKEN = '_password_reset_token'
class PasswordResetDoneView(PasswordContextMixin, TemplateView): class PasswordResetDoneView(PasswordContextMixin, TemplateView):
template_name = 'registration/password_reset_done.html' template_name = 'registration/password_reset_done.html'
title = _('Password reset sent') title = _('Password reset sent')
@ -446,12 +450,26 @@ class PasswordResetConfirmView(PasswordContextMixin, FormView):
self.validlink = False self.validlink = False
self.user = self.get_user(kwargs['uidb64']) self.user = self.get_user(kwargs['uidb64'])
if self.user is not None and self.token_generator.check_token(self.user, kwargs['token']): if self.user is not None:
self.validlink = True token = kwargs['token']
else: if token == INTERNAL_RESET_URL_TOKEN:
return self.render_to_response(self.get_context_data()) 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)
return super(PasswordResetConfirmView, self).dispatch(*args, **kwargs) # Display the "Password reset unsuccessful" page.
return self.render_to_response(self.get_context_data())
def get_user(self, uidb64): def get_user(self, uidb64):
try: try:
@ -471,6 +489,7 @@ class PasswordResetConfirmView(PasswordContextMixin, FormView):
user = form.save() user = form.save()
if self.post_reset_login: if self.post_reset_login:
auth_login(self.request, user) auth_login(self.request, user)
del self.request.session[INTERNAL_RESET_SESSION_TOKEN]
return super(PasswordResetConfirmView, self).form_valid(form) return super(PasswordResetConfirmView, self).form_valid(form)
def get_context_data(self, **kwargs): def get_context_data(self, **kwargs):

View File

@ -116,6 +116,14 @@ Minor features
:class:`~django.contrib.auth.views.PasswordResetConfirmView` allows :class:`~django.contrib.auth.views.PasswordResetConfirmView` allows
automatically logging in a user after a successful password reset. 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 * :func:`~django.contrib.auth.update_session_auth_hash` now rotates the session
key to allow a password change to invalidate stolen session cookies. key to allow a password change to invalidate stolen session cookies.

View File

@ -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)

View File

@ -3,12 +3,15 @@ from django.contrib.auth.models import User
from django.contrib.auth.tokens import PasswordResetTokenGenerator from django.contrib.auth.tokens import PasswordResetTokenGenerator
from django.contrib.auth.views import ( from django.contrib.auth.views import (
PasswordChangeDoneView, PasswordChangeView, PasswordResetCompleteView, PasswordChangeDoneView, PasswordChangeView, PasswordResetCompleteView,
PasswordResetConfirmView, PasswordResetDoneView, PasswordResetView, PasswordResetDoneView, PasswordResetView,
) )
from django.test import RequestFactory, TestCase, override_settings 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.encoding import force_bytes, force_text
from django.utils.http import urlsafe_base64_encode from django.utils.http import urlsafe_base64_encode
from .client import PasswordResetConfirmClient
@override_settings(ROOT_URLCONF='auth_tests.urls') @override_settings(ROOT_URLCONF='auth_tests.urls')
class AuthTemplateTests(TestCase): class AuthTemplateTests(TestCase):
@ -34,16 +37,20 @@ class AuthTemplateTests(TestCase):
def test_PasswordResetConfirmView_invalid_token(self): def test_PasswordResetConfirmView_invalid_token(self):
# PasswordResetConfirmView invalid token # 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, '<title>Password reset unsuccessful</title>')
self.assertContains(response, '<h1>Password reset unsuccessful</h1>') self.assertContains(response, '<h1>Password reset unsuccessful</h1>')
def test_PasswordResetConfirmView_valid_token(self): def test_PasswordResetConfirmView_valid_token(self):
# PasswordResetConfirmView valid token # PasswordResetConfirmView valid token
client = PasswordResetConfirmClient()
default_token_generator = PasswordResetTokenGenerator() default_token_generator = PasswordResetTokenGenerator()
token = default_token_generator.make_token(self.user) token = default_token_generator.make_token(self.user)
uidb64 = force_text(urlsafe_base64_encode(force_bytes(self.user.pk))) 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, '<title>Enter new password</title>')
self.assertContains(response, '<h1>Enter new password</h1>') self.assertContains(response, '<h1>Enter new password</h1>')

View File

@ -62,3 +62,10 @@ class TokenGeneratorTest(TestCase):
# This will put a 14-digit base36 timestamp into the token, which is too large. # This will put a 14-digit base36 timestamp into the token, which is too large.
with self.assertRaises(ValueError): with self.assertRaises(ValueError):
p0._make_token_with_timestamp(user, 175455491841851871349) 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)

View File

@ -16,7 +16,8 @@ from django.contrib.auth.forms import (
) )
from django.contrib.auth.models import User from django.contrib.auth.models import User
from django.contrib.auth.views import ( 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.sessions.middleware import SessionMiddleware
from django.contrib.sites.requests import RequestSite from django.contrib.sites.requests import RequestSite
@ -24,7 +25,7 @@ from django.core import mail
from django.db import connection from django.db import connection
from django.http import HttpRequest, QueryDict from django.http import HttpRequest, QueryDict
from django.middleware.csrf import CsrfViewMiddleware, get_token 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.test.utils import patch_logger
from django.urls import NoReverseMatch, reverse, reverse_lazy from django.urls import NoReverseMatch, reverse, reverse_lazy
from django.utils.deprecation import RemovedInDjango21Warning 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.six.moves.urllib.parse import ParseResult, urlparse
from django.utils.translation import LANGUAGE_SESSION_KEY from django.utils.translation import LANGUAGE_SESSION_KEY
from .client import PasswordResetConfirmClient
from .models import CustomUser, UUIDUser from .models import CustomUser, UUIDUser
from .settings import AUTH_TEMPLATES from .settings import AUTH_TEMPLATES
@ -116,6 +118,9 @@ class AuthViewNamedURLTests(AuthViewsTestCase):
class PasswordResetTest(AuthViewsTestCase): class PasswordResetTest(AuthViewsTestCase):
def setUp(self):
self.client = PasswordResetConfirmClient()
def test_email_not_found(self): def test_email_not_found(self):
"""If the provided email is not registered, don't raise any error but """If the provided email is not registered, don't raise any error but
also don't send any email.""" also don't send any email."""
@ -278,6 +283,8 @@ class PasswordResetTest(AuthViewsTestCase):
# Check the password has been changed # Check the password has been changed
u = User.objects.get(email='staffmember@example.com') u = User.objects.get(email='staffmember@example.com')
self.assertTrue(u.check_password("anewpassword")) 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 # Check we can't use the link again
response = self.client.get(path) response = self.client.get(path)
@ -338,6 +345,23 @@ class PasswordResetTest(AuthViewsTestCase):
response = self.client.get('/reset/zzzzzzzzzzzzz/1-1/') response = self.client.get('/reset/zzzzzzzzzzzzz/1-1/')
self.assertContains(response, "Hello, .") 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') @override_settings(AUTH_USER_MODEL='auth_tests.CustomUser')
class CustomUserPasswordResetTest(AuthViewsTestCase): class CustomUserPasswordResetTest(AuthViewsTestCase):
@ -352,6 +376,9 @@ class CustomUserPasswordResetTest(AuthViewsTestCase):
cls.u1.set_password('password') cls.u1.set_password('password')
cls.u1.save() cls.u1.save()
def setUp(self):
self.client = PasswordResetConfirmClient()
def _test_confirm_start(self): def _test_confirm_start(self):
# Start by creating the email # Start by creating the email
response = self.client.post('/password_reset/', {'email': self.user_email}) response = self.client.post('/password_reset/', {'email': self.user_email})