Skip to content

Commit

Permalink
Fixed potentional security issue with leaked password tokens (#1757)
Browse files Browse the repository at this point in the history
* Fixed potentional security issue with leaked password tokens
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.

* Fix tests

* Fix redirect bug in test

* Fix lint errors

* Fix lint
  • Loading branch information
joshblum authored and pennersr committed Jul 26, 2017
1 parent 1e526ed commit 7c2b13c
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 14 deletions.
9 changes: 8 additions & 1 deletion allauth/account/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,10 @@ def test_password_reset_flow(self):
# Extract URL for `password_reset_from_key` view and access it
url = body[body.find('/password/reset/'):].split()[0]
resp = self.client.get(url)
# Follow the redirect the actual password reset page with the key
# hidden.
url = resp.url
resp = self.client.get(url)
self.assertTemplateUsed(
resp,
'account/password_reset_from_key.%s' %
Expand Down Expand Up @@ -336,8 +340,11 @@ def test_password_reset_ACCOUNT_LOGIN_ON_PASSWORD_RESET(self):
user = self._request_new_password()
body = mail.outbox[0].body
url = body[body.find('/password/reset/'):].split()[0]
resp = self.client.get(url)
# Follow the redirect the actual password reset page with the key
# hidden.
resp = self.client.post(
url,
resp.url,
{'password1': 'newpass123',
'password2': 'newpass123'})
self.assertTrue(is_authenticated(user))
Expand Down
46 changes: 33 additions & 13 deletions allauth/account/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@
)


INTERNAL_RESET_URL_KEY = "set-password"
INTERNAL_RESET_SESSION_KEY = "_password_reset_key"


sensitive_post_parameters_m = method_decorator(
sensitive_post_parameters('password', 'password1', 'password2'))

Expand All @@ -61,6 +65,7 @@ def _ajax_response(request, response, form=None, data=None):


class RedirectAuthenticatedUserMixin(object):

def dispatch(self, request, *args, **kwargs):
if is_authenticated(request.user) and \
app_settings.AUTHENTICATED_LOGIN_REDIRECTS:
Expand Down Expand Up @@ -671,21 +676,36 @@ def get_form_class(self):
def dispatch(self, request, uidb36, key, **kwargs):
self.request = request
self.key = key
# (Ab)using forms here to be able to handle errors in XHR #890
token_form = UserTokenForm(data={'uidb36': uidb36, 'key': key})

if not token_form.is_valid():
self.reset_user = None
response = self.render_to_response(
self.get_context_data(token_fail=True)
)
return _ajax_response(self.request, response, form=token_form)
if self.key == INTERNAL_RESET_URL_KEY:
self.key = self.request.session.get(INTERNAL_RESET_SESSION_KEY, '')
# (Ab)using forms here to be able to handle errors in XHR #890
token_form = UserTokenForm(
data={'uidb36': uidb36, 'key': self.key})
if token_form.is_valid():
self.reset_user = token_form.reset_user
return super(PasswordResetFromKeyView, self).dispatch(request,
uidb36,
self.key,
**kwargs)
else:
self.reset_user = token_form.reset_user
return super(PasswordResetFromKeyView, self).dispatch(request,
uidb36,
key,
**kwargs)
token_form = UserTokenForm(
data={'uidb36': uidb36, 'key': self.key})
if token_form.is_valid():
# Store the key in the session and redirect to the
# password reset form at a URL without the key. That
# avoids the possibility of leaking the key in the
# HTTP Referer header.
self.request.session[INTERNAL_RESET_SESSION_KEY] = self.key
redirect_url = self.request.path.replace(
self.key, INTERNAL_RESET_URL_KEY)
return redirect(redirect_url)

self.reset_user = None
response = self.render_to_response(
self.get_context_data(token_fail=True)
)
return _ajax_response(self.request, response, form=token_form)

def get_context_data(self, **kwargs):
ret = super(PasswordResetFromKeyView, self).get_context_data(**kwargs)
Expand Down

0 comments on commit 7c2b13c

Please sign in to comment.