diff --git a/openedx/core/djangoapps/user_authn/views/password_reset.py b/openedx/core/djangoapps/user_authn/views/password_reset.py index 83815243b999962551080fef34db29aa7a71c36e..692ddc0a00499dac528012718c13e40ae943ecf5 100644 --- a/openedx/core/djangoapps/user_authn/views/password_reset.py +++ b/openedx/core/djangoapps/user_authn/views/password_reset.py @@ -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 diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_password.py b/openedx/core/djangoapps/user_authn/views/tests/test_password.py index f1c9666dc6a3982ae25d0ea6105582efb43f0b3e..68ffb0095ddcb755c8177e8d5e2a6ac0351b0884 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_password.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_password.py @@ -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 diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py b/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py index 7ac768e9fb6c9a407baea4f0f89ebb834c870469..2e4b3b6d28fe93ffbad347021ee18f29e20f7783 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py @@ -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.