Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix password reset token leakage in referrer

Browse Source
This commit is contained in:
DawoudSheraz
2020-09-17 12:36:13 +05:00
parent d74f6d9800
commit 6181edfa4c
3 changed files with 66 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

35
openedx/core/djangoapps/user_authn/views/password_reset.py
View File

@@ -312,8 +312,6 @@ class PasswordResetConfirmWrapper(PasswordResetConfirmView):
We also optionally do some additional password policy checks.
"""
reset_url_token = 'set-password'
def __init__(self):
self.platform_name = PasswordResetConfirmWrapper._get_platform_name()
self.validlink = False
@@ -336,15 +334,11 @@ class PasswordResetConfirmWrapper(PasswordResetConfirmView):
context.update(extra_context)
return self.render_to_response(context)
def _set_token_in_session(self, request, token):
def _get_token_from_session(self, request):
"""
method to store password reset token in session received in reset password url
Internal method to get password reset token from session.
"""
if not token:
return
session = request.session
session[INTERNAL_RESET_SESSION_TOKEN] = token
session.save()
return request.session[INTERNAL_RESET_SESSION_TOKEN]
@staticmethod
def _get_platform_name():
@@ -490,16 +484,23 @@ class PasswordResetConfirmWrapper(PasswordResetConfirmView):
return self._handle_retired_user(self.request)
if self.request.method == 'POST':
# Get actual token from session before processing the POST request.
# This is needed because django's post process is not called on password reset
# post request and the correct token needs to be extracted from session.
self.token = self._get_token_from_session(self.request)
return self.post(self.request, *args, **kwargs)
else:
self._set_token_in_session(self.request, self.token)
token = self.reset_url_token
response = super(PasswordResetConfirmWrapper, self).dispatch(self.request, uidb64=self.uidb64, token=token,
extra_context=self.platform_name)
response_was_successful = response.context_data.get('validlink')
if response_was_successful and not self.user.is_active:
self.user.is_active = True
self.user.save()
response = super(PasswordResetConfirmWrapper, self).dispatch(
self.request,
uidb64=self.uidb64,
token=self.token,
extra_context=self.platform_name
)
if hasattr(response, 'context_data'):
response_was_successful = response.context_data.get('validlink')
if response_was_successful and not self.user.is_active:
self.user.is_active = True
self.user.save()
return response

7
openedx/core/djangoapps/user_authn/views/tests/test_password.py
View File

@@ -133,11 +133,14 @@ class TestPasswordChange(CreateAccountMixin, CacheIsolationTestCase):
# Visit the activation link
response = self.client.get(activation_link)
self.assertEqual(response.status_code, 200)
self.assertEqual(response.status_code, 302)
# Visit the redirect link
_ = self.client.get(response.url)
# Submit a new password and follow the redirect to the success page
response = self.client.post(
activation_link,
response.url,
# These keys are from the form on the current password reset confirmation page.
{'new_password1': self.NEW_PASSWORD, 'new_password2': self.NEW_PASSWORD},
follow=True

48
openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py
View File

@@ -78,6 +78,13 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
self.user_bad_passwd.password = UNUSABLE_PASSWORD_PREFIX
self.user_bad_passwd.save()
def setup_request_session_with_token(self, request):
"""
Internal helper to setup request session and add token in session.
"""
process_request(request)
request.session[INTERNAL_RESET_SESSION_TOKEN] = self.token
@patch(
'openedx.core.djangoapps.user_authn.views.password_reset.render_to_string',
Mock(side_effect=mock_render_to_string, autospec=True)
@@ -388,7 +395,15 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
def test_reset_password_good_token(self):
"""
Tests good token and uidb36 in password reset
Tests good token and uidb36 in password reset.
Scenario:
When the password reset url is opened
Then the page is redirected to url without token
And token gets set in session
When the redirected page is visited with token in session
Then reset password page renders
And inactive user is set to active
"""
url = reverse(
"password_reset_confirm",
@@ -397,13 +412,28 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
good_reset_req = self.request_factory.get(url)
process_request(good_reset_req)
good_reset_req.user = self.user
PasswordResetConfirmWrapper.as_view()(good_reset_req, uidb36=self.uidb36, token=self.token)
redirect_response = PasswordResetConfirmWrapper.as_view()(good_reset_req, uidb36=self.uidb36, token=self.token)
good_reset_req = self.request_factory.get(redirect_response.url)
self.setup_request_session_with_token(good_reset_req)
good_reset_req.user = self.user
# set-password is the new token representation in the redirect url
PasswordResetConfirmWrapper.as_view()(good_reset_req, uidb36=self.uidb36, token='set-password')
self.user = User.objects.get(pk=self.user.pk)
self.assertTrue(self.user.is_active)
def test_reset_password_good_token_with_anonymous_user(self):
"""
Tests good token and uidb36 in password reset for anonymous user
Tests good token and uidb36 in password reset for anonymous user.
Scenario:
When the password reset url is opened with anonymous user in request
Then the page is redirected to url without token
And token gets set in session
When the redirected page is visited with token in session
Then reset password page renders
And inactive user associated with token is set to active
"""
url = reverse(
"password_reset_confirm",
@@ -412,7 +442,14 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
good_reset_req = self.request_factory.get(url)
process_request(good_reset_req)
good_reset_req.user = AnonymousUser()
PasswordResetConfirmWrapper.as_view()(good_reset_req, uidb36=self.uidb36, token=self.token)
redirect_response = PasswordResetConfirmWrapper.as_view()(good_reset_req, uidb36=self.uidb36, token=self.token)
good_reset_req = self.request_factory.get(redirect_response.url)
self.setup_request_session_with_token(good_reset_req)
good_reset_req.user = AnonymousUser()
# set-password is the new token representation in the redirect url
PasswordResetConfirmWrapper.as_view()(good_reset_req, uidb36=self.uidb36, token='set-password')
self.user = User.objects.get(pk=self.user.pk)
self.assertTrue(self.user.is_active)
@@ -428,7 +465,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
)
request_params = {'new_password1': 'password1', 'new_password2': 'password2'}
confirm_request = self.request_factory.post(url, data=request_params)
process_request(confirm_request)
self.setup_request_session_with_token(confirm_request)
confirm_request.user = self.user
# Make a password reset request with mismatching passwords.
@@ -512,6 +549,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
)
request_params = {'new_password1': password_dict['password'], 'new_password2': password_dict['password']}
confirm_request = self.request_factory.post(url, data=request_params)
self.setup_request_session_with_token(confirm_request)
confirm_request.user = self.user
# Make a password reset request with minimum/maximum passwords characters.