Fixed password reset for authenticated user.

Password reset link for one user is working for other logged in users.
Fixed by verifying the token with request.user if authenticated.

LEARNER-5114
This commit is contained in:
Waheed Ahmed
2019-08-29 18:10:23 +05:00
parent 85f622f0e1
commit 9f80fd2b87
2 changed files with 44 additions and 6 deletions

View File

@@ -11,10 +11,11 @@ import unittest
import ddt
from django.conf import settings
from django.contrib.auth.hashers import UNUSABLE_PASSWORD_PREFIX, make_password
from django.contrib.auth.models import User
from django.contrib.auth.models import AnonymousUser, User
from django.contrib.auth.tokens import default_token_generator
from django.core import mail
from django.core.cache import cache
from django.http import Http404
from django.test.client import RequestFactory
from django.test.utils import override_settings
from django.urls import reverse
@@ -286,6 +287,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
kwargs={"uidb36": uidb36, "token": token}
)
)
bad_request.user = AnonymousUser()
password_reset_confirm_wrapper(bad_request, uidb36, token)
self.user = User.objects.get(pk=self.user.pk)
self.assertFalse(self.user.is_active)
@@ -299,6 +301,21 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
kwargs={"uidb36": self.uidb36, "token": self.token}
)
good_reset_req = self.request_factory.get(url)
good_reset_req.user = self.user
password_reset_confirm_wrapper(good_reset_req, self.uidb36, self.token)
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
"""
url = reverse(
"password_reset_confirm",
kwargs={"uidb36": self.uidb36, "token": self.token}
)
good_reset_req = self.request_factory.get(url)
good_reset_req.user = AnonymousUser()
password_reset_confirm_wrapper(good_reset_req, self.uidb36, self.token)
self.user = User.objects.get(pk=self.user.pk)
self.assertTrue(self.user.is_active)
@@ -315,6 +332,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
)
request_params = {'new_password1': 'password1', 'new_password2': 'password2'}
confirm_request = self.request_factory.post(url, data=request_params)
confirm_request.user = self.user
# Make a password reset request with mismatching passwords.
resp = password_reset_confirm_wrapper(confirm_request, self.uidb36, self.token)
@@ -338,6 +356,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
kwargs={'uidb36': self.uidb36, 'token': self.token}
)
reset_req = self.request_factory.get(url)
reset_req.user = self.user
resp = password_reset_confirm_wrapper(reset_req, self.uidb36, self.token)
# Verify the response status code is: 200 with password reset fail and also verify that
@@ -352,6 +371,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
kwargs={"uidb36": self.uidb36, "token": self.token}
)
for request in [self.request_factory.get(url), self.request_factory.post(url)]:
request.user = self.user
response = password_reset_confirm_wrapper(request, self.uidb36, self.token)
assert response.context_data['err_msg'] == SYSTEM_MAINTENANCE_MSG
self.user.refresh_from_db()
@@ -371,7 +391,8 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
password = u'p\u212bssword'
request_params = {'new_password1': password, 'new_password2': password}
confirm_request = self.request_factory.post(url, data=request_params)
response = password_reset_confirm_wrapper(confirm_request, self.uidb36, self.token)
confirm_request.user = self.user
__ = password_reset_confirm_wrapper(confirm_request, self.uidb36, self.token)
user = User.objects.get(pk=self.user.pk)
salt_val = user.password.split('$')[1]
@@ -404,6 +425,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)
confirm_request.user = self.user
# Make a password reset request with minimum/maximum passwords characters.
response = password_reset_confirm_wrapper(confirm_request, self.uidb36, self.token)
@@ -421,6 +443,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
kwargs={"uidb36": self.uidb36, "token": self.token}
)
good_reset_req = self.request_factory.get(url)
good_reset_req.user = self.user
password_reset_confirm_wrapper(good_reset_req, self.uidb36, self.token)
confirm_kwargs = reset_confirm.call_args[1]
self.assertEquals(confirm_kwargs['extra_context']['platform_name'], 'Fake University')
@@ -445,3 +468,16 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
subj, _, _, _ = send_email.call_args[0]
self.assertIn(platform_name, subj)
def test_reset_password_with_other_user_link(self):
"""
Tests that user should not be able to reset password through other user's token
"""
reset_url = reverse(
"password_reset_confirm",
kwargs={"uidb36": self.uidb36, "token": self.token}
)
reset_request = self.request_factory.get(reset_url)
reset_request.user = UserFactory.create()
self.assertRaises(Http404, password_reset_confirm_wrapper, reset_request, self.uidb36, self.token)

View File

@@ -137,8 +137,8 @@ def index(request, extra_context=None, user=AnonymousUser()):
courses = get_courses(user)
if configuration_helpers.get_value(
"ENABLE_COURSE_SORTING_BY_START_DATE",
settings.FEATURES["ENABLE_COURSE_SORTING_BY_START_DATE"],
"ENABLE_COURSE_SORTING_BY_START_DATE",
settings.FEATURES["ENABLE_COURSE_SORTING_BY_START_DATE"],
):
courses = sort_by_start_date(courses)
else:
@@ -685,7 +685,6 @@ def password_change_request_handler(request):
# no user associated with the email
if configuration_helpers.get_value('ENABLE_PASSWORD_RESET_FAILURE_EMAIL',
settings.FEATURES['ENABLE_PASSWORD_RESET_FAILURE_EMAIL']):
site = get_current_site()
message_context = get_base_template_context(site)
@@ -809,6 +808,9 @@ def password_reset_confirm_wrapper(request, uidb36=None, token=None):
try:
uid_int = base36_to_int(uidb36)
if request.user.is_authenticated and request.user.id != uid_int:
raise Http404
user = User.objects.get(id=uid_int)
except (ValueError, User.DoesNotExist):
# if there's any error getting a user, just let django's
@@ -1167,7 +1169,7 @@ def confirm_email_change(request, key): # pylint: disable=unused-argument
# Send it to the old email...
try:
ace.send(msg)
except Exception: # pylint: disable=broad-except
except Exception: # pylint: disable=broad-except
log.warning('Unable to send confirmation email to old address', exc_info=True)
response = render_to_response("email_change_failed.html", {'email': user.email})
transaction.set_rollback(True)