Skip to content

Commit

Permalink
Fixed potentional security issue with leaked password tokens
Browse files Browse the repository at this point in the history
Django 1.11+ prevents password tokens from being leaked through the
HTTP Referer header if a template calls out to third-party resources
(i.e., JS or CSS) by setting token in session and redirecting.

This change brings that change to PasswordResetTokenView.

Fixes #258
  • Loading branch information
brosner committed Jun 9, 2017
1 parent 0ed4328 commit 45aef3d
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 10 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@
BI indicates a backward incompatible change. Take caution when upgrading to a
version with these. Your code will need to be updated to continue working.

## 2.0.2

* fixed potentional security issue with leaking password reset tokens through HTTP Referer header

## 2.0.1

@@@ todo

## 2.0.0

* BI: moved account deletion callbacks to hooksets
Expand Down
1 change: 1 addition & 0 deletions account/tests/templates/account/email/password_reset.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{{ password_reset_url }}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Hello
1 change: 1 addition & 0 deletions account/tests/templates/account/password_reset_sent.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# empty for now
1 change: 1 addition & 0 deletions account/tests/templates/account/password_reset_token.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# empty for now
67 changes: 67 additions & 0 deletions account/tests/test_views.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from django.conf import settings
from django.core import mail
from django.test import TestCase, override_settings
from django.utils.http import int_to_base36
from django.utils.six.moves.urllib.parse import urlparse

from django.contrib.auth.models import User

Expand Down Expand Up @@ -348,3 +350,68 @@ def test_post_authenticated_success_no_mail(self):
fetch_redirect_response=False
)
self.assertEqual(len(mail.outbox), 0)


class PasswordResetTokenViewTestCase(TestCase):

def signup(self):
data = {
"username": "foo",
"password": "bar",
"password_confirm": "bar",
"email": "foobar@example.com",
"code": "abc123",
}
self.client.post(reverse("account_signup"), data)
mail.outbox = []
return User.objects.get(username="foo")

def request_password_reset(self):
user = self.signup()
data = {
"email": user.email,
}
self.client.post(reverse("account_password_reset"), data)
parsed = urlparse(mail.outbox[0].body.strip())
return user, parsed.path

def test_get_bad_user(self):
url = reverse(
"account_password_reset_token",
kwargs={
"uidb36": int_to_base36(100),
"token": "notoken",
}
)
response = self.client.get(url)
self.assertEqual(response.status_code, 404)

def test_get_reset(self):
user, url = self.request_password_reset()
response = self.client.get(url)
self.assertRedirects(
response,
reverse(
"account_password_reset_token",
kwargs={
"uidb36": int_to_base36(user.id),
"token": "set-password",
}
),
fetch_redirect_response=False
)

def test_post_reset(self):
user, url = self.request_password_reset()
response = self.client.get(url)
self.assertEqual(response.status_code, 302)
data = {
"password": "new-password",
"password_confirm": "new-password",
}
response = self.client.post(response["Location"], data)
self.assertRedirects(
response,
reverse(settings.ACCOUNT_PASSWORD_RESET_REDIRECT_URL),
fetch_redirect_response=False
)
33 changes: 23 additions & 10 deletions account/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,10 @@ def make_token(self, user):
return self.token_generator.make_token(user)


INTERNAL_RESET_URL_TOKEN = "set-password"
INTERNAL_RESET_SESSION_TOKEN = "_password_reset_token"


class PasswordResetTokenView(PasswordMixin, FormView):

template_name = "account/password_reset_token.html"
Expand All @@ -620,22 +624,31 @@ class PasswordResetTokenView(PasswordMixin, FormView):
form_password_field = "password"
fallback_url_setting = "ACCOUNT_PASSWORD_RESET_REDIRECT_URL"

def dispatch(self, *args, **kwargs):
user = self.get_user()
if 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.check_token(user, session_token):
return super(PasswordResetTokenView, self).dispatch(*args, **kwargs)
else:
if self.check_token(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 redirect(redirect_url)
return self.token_fail()

def get(self, request, **kwargs):
form_class = self.get_form_class()
form = self.get_form(form_class)
ctx = self.get_context_data(form=form)
if not self.check_token(self.get_user(), self.kwargs["token"]):
return self.token_fail()
return self.render_to_response(ctx)

def get_context_data(self, **kwargs):
ctx = super(PasswordResetTokenView, self).get_context_data(**kwargs)
ctx.update({
"uidb36": self.kwargs["uidb36"],
"token": self.kwargs["token"],
})
return ctx

def form_valid(self, form):
self.change_password(form)
self.create_password_history(form, self.request.user)
Expand Down

0 comments on commit 45aef3d

Please sign in to comment.