Merge pull request #25214 from edx/private_to_public_6f254aa

Mergeback PR from private to public.

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
 
 

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

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.