ENH Refactor password reset to support SameSite=Strict

Performing an immediate redirect on a request from an external website,
such as a web-based email client, causes the new request to be treated
as external, and when the session cookie is set to Samesite=Strict, this
prevents the cookie from being sent by the browser, triggering a fresh
session. This meant that the existing password reset mechanism would
not function in this mode, as the AutoLoginHash was being stored in the
session and immediately lost, triggering a redirect to the login form.

This change refactors the change password handler to instead push the
AutoLoginHash value into the change password form as a hidden field,
ensuring it can be read during submission.

It also includes broader test coverage of the change password handler,
though this remains incomplete due to time constraints.

src/Security/MemberAuthenticator/ChangePasswordForm.php

@@ -79,4 +79,14 @@ protected function getFormActions()
 
         return $actions;
     }
+
+    /**
+     * Adds a hidden field to the form with the given autologin hash, to support the "forgot password" workflow.
+     */
+    public function addAutoLoginHash(string $hash): self
+    {
+        $this->fields->push(HiddenField::create('AutoLoginHash', 'AutoLoginHash', $hash));
+
+        return $this;
+    }
 }

src/Security/MemberAuthenticator/ChangePasswordHandler.php

@@ -75,14 +75,6 @@ public function changepassword()
 
         // Check whether we are merely changing password, or resetting.
         if ($token !== null && $member && $member->validateAutoLoginToken($token)) {
-            $this->setSessionToken($member, $token);
-
-            // Redirect to myself, but without the hash in the URL
-            return $this->redirect($this->link);
-        }
-
-        $session = $this->getRequest()->getSession();
-        if ($session->get('AutoLoginHash')) {
             $message = DBField::create_field(
                 'HTMLFragment',
                 '<p>' . _t(
@@ -91,10 +83,12 @@ public function changepassword()
                 ) . '</p>'
             );
 
-            // Subsequent request after the "first load with hash" (see previous if clause).
+            $form = $this->changePasswordForm();
+            $form->addAutoLoginHash($member->encryptWithUserSettings($token));
+
             return [
                 'Content' => $message,
-                'Form'    => $this->changePasswordForm()
+                'Form'    => $form,
             ];
         }
 
@@ -110,7 +104,7 @@ public function changepassword()
 
             return [
                 'Content' => $message,
-                'Form'    => $this->changePasswordForm()
+                'Form'    => $this->changePasswordForm(),
             ];
         }
         // Show a friendly message saying the login token has expired
@@ -144,23 +138,6 @@ public function changepassword()
         );
     }
 
-
-    /**
-     * @param Member $member
-     * @param string $token
-     */
-    protected function setSessionToken($member, $token)
-    {
-        // if there is a current member, they should be logged out
-        if ($curMember = Security::getCurrentUser()) {
-            Injector::inst()->get(IdentityStore::class)->logOut();
-        }
-
-        $this->getRequest()->getSession()->regenerateSessionId();
-        // Store the hash for the change password form. Will be unset after reload within the ChangePasswordForm.
-        $this->getRequest()->getSession()->set('AutoLoginHash', $member->encryptWithUserSettings($token));
-    }
-
     /**
      * Return a link to this request handler.
      * The link returned is supplied in the constructor
@@ -215,16 +192,13 @@ public function doChangePassword(array $data, $form)
             return $this->redirectBackToForm();
         }
 
-        $session = $this->getRequest()->getSession();
         if (!$member) {
-            if ($session->get('AutoLoginHash')) {
-                $member = Member::member_from_autologinhash($session->get('AutoLoginHash'));
+            if (isset($data['AutoLoginHash'])) {
+                $member = Member::member_from_autologinhash($data['AutoLoginHash']);
             }
 
-            // The user is not logged in and no valid auto login hash is available
+            // The user is not logged in and no valid token was provided
             if (!$member) {
-                $session->clear('AutoLoginHash');
-
                 return $this->redirect($this->addBackURLParam(Security::singleton()->Link('login')));
             }
         }
@@ -240,7 +214,7 @@ public function doChangePassword(array $data, $form)
             );
 
             // redirect back to the form, instead of using redirectBack() which could send the user elsewhere.
-            return $this->redirectBackToForm();
+            return $this->redirectBackToForm($member->ID, $data['AutoLoginHash']);
         }
 
         // Fail if passwords do not match
@@ -254,15 +228,15 @@ public function doChangePassword(array $data, $form)
             );
 
             // redirect back to the form, instead of using redirectBack() which could send the user elsewhere.
-            return $this->redirectBackToForm();
+            return $this->redirectBackToForm($member->ID, $data['AutoLoginHash']);
         }
 
         // Check if the new password is accepted
         $validationResult = $member->changePassword($data['NewPassword1']);
         if (!$validationResult->isValid()) {
             $form->setSessionValidationResult($validationResult);
 
-            return $this->redirectBackToForm();
+            return $this->redirectBackToForm($member->ID, $data['AutoLoginHash']);
         }
 
         // Clear locked out status
@@ -293,8 +267,6 @@ public function doChangePassword(array $data, $form)
             $identityStore->logIn($member, false, $this->getRequest());
         }
 
-        $session->clear('AutoLoginHash');
-
         // Redirect to backurl
         $backURL = $this->getBackURL();
         if ($backURL
@@ -319,10 +291,18 @@ public function doChangePassword(array $data, $form)
      *
      * @return HTTPResponse
      */
-    public function redirectBackToForm()
+    public function redirectBackToForm(?int $withMemberID = null, ?string $withToken = null)
     {
         // Redirect back to form
-        $url = $this->addBackURLParam(Security::singleton()->Link('changepassword'));
+        $url = Security::singleton()->Link('changepassword');
+
+        // Include token data if performing an unauthenticated password reset
+        if ($withMemberID && $withToken) {
+            $url = Controller::join_links($url, "?m={$withMemberID}&t={$withToken}");
+        }
+
+        // Add Back URL if available
+        $url = $this->addBackURLParam($url);
 
         return $this->redirect($url);
     }

tests/php/Security/MemberAuthenticator/ChangePasswordHandlerTest.php

@@ -10,8 +10,9 @@
 use SilverStripe\Security\MemberAuthenticator\ChangePasswordHandler;
 use SilverStripe\Security\MemberAuthenticator\MemberAuthenticator;
 use SilverStripe\Security\Security;
+use SilverStripe\Dev\FunctionalTest;
 
-class ChangePasswordHandlerTest extends SapphireTest
+class ChangePasswordHandlerTest extends FunctionalTest
 {
     protected static $fixture_file = 'ChangePasswordHandlerTest.yml';
 
@@ -46,4 +47,50 @@ public function testExpiredOrInvalidTokenProvidesLostPasswordAndLoginLink()
         $this->assertStringContainsString('Security/lostpassword', $result['Content'], 'Lost password URL is included');
         $this->assertStringContainsString('Security/login', $result['Content'], 'Login URL is included');
     }
+
+    public function testLegitimateTokenLoadsChangePasswordForm()
+    {
+        $member = $this->objFromFixture(Member::class, 'sarah');
+        $token = $member->generateAutologinTokenAndStoreHash();
+        $hash = $member->AutoLoginHash;
+
+        $request = new HTTPRequest('GET', '/Security/changepassword', [
+            'm' => $member->ID,
+            't' => $token,
+        ]);
+        $request->setSession(new Session([]));
+
+        /** @var ChangePasswordHandler $handler */
+        $handler = $this->getMockBuilder(ChangePasswordHandler::class)
+            ->disableOriginalConstructor()
+            ->setMethods(null)
+            ->getMock();
+
+        $result = $handler->setRequest($request)->changepassword();
+
+        $this->assertIsArray($result, 'An array is returned');
+        $this->assertArrayHasKey('Form', $result, 'Form is included');
+
+        $hashField = $result['Form']->HiddenFields()->dataFieldByName('AutoLoginHash') ?? null;
+        $this->assertIsObject($hashField, 'AutoLoginHash field is included');
+        $this->assertEquals($hashField->value, $hash, 'AutoLoginHash field value is correct');
+    }
+
+    public function testSubmittingChangePasswordFormSucceedsWithValidToken()
+    {
+        $member = $this->objFromFixture(Member::class, 'sarah');
+        $hash = $member->generateAutologinTokenAndStoreHash();
+
+        $this->get("/Security/changepassword?m={$member->ID}&t={$hash}");
+        $result = $this->submitForm('ChangePasswordForm_ChangePasswordForm', null, [
+            'NewPassword1' => 'newpassword',
+            'NewPassword2' => 'newpassword',
+        ]);
+
+        $this->assertStringContainsString(
+            'You're logged in',
+            $result->getBody(),
+            'User is logged in'
+        );
+    }
 }

tests/php/Security/MemberAuthenticator/ChangePasswordHandlerTest.yml

@@ -2,4 +2,3 @@ SilverStripe\Security\Member:
   sarah:
     FirstName: Sarah
     Surname: Smith
-    AutoLoginToken: foobar