ENGCOM-8461: Extend password reset token validity on password change page load #25279

magento-engcom-team · magento-engcom-team · commit b5501078d75c · 2020-11-27T10:07:25.000-06:00
- Merge Pull Request #25279 from fredden/magento2:password-reset/race-form-fill - Merged commits: 1. c7fc6d5 2. ede673f 3. c19a1a1 4. 8cd951a 5. 9c15c95 6. 35a8976 7. defadd0 8. 5c43558 9. c3f3a57 10. 03c65e9 11. e82531d 12. 0a4176e 13. eba286f 14. 42a3eb7 15. 1518f8c 16. 7aef712 17. 35b5ad8 18. dfe2370 19. c0cbe80
diff --git a/app/code/Magento/Customer/Controller/Account/CreatePassword.php b/app/code/Magento/Customer/Controller/Account/CreatePassword.php
@@ -9,21 +9,22 @@
 
 use Magento\Customer\Api\AccountManagementInterface;
 use Magento\Customer\Model\ForgotPasswordToken\ConfirmCustomerByToken;
+use Magento\Customer\Model\ForgotPasswordToken\GetCustomerByToken;
 use Magento\Customer\Model\Session;
-use Magento\Framework\App\Action\HttpGetActionInterface;
-use Magento\Framework\View\Result\PageFactory;
 use Magento\Framework\App\Action\Context;
+use Magento\Framework\App\Action\HttpGetActionInterface;
 use Magento\Framework\App\ObjectManager;
+use Magento\Framework\Controller\Result\Redirect;
+use Magento\Framework\View\Result\Page;
+use Magento\Framework\View\Result\PageFactory;
 
 /**
- * Class CreatePassword
- *
- * @package Magento\Customer\Controller\Account
+ * Controller for front-end customer password reset form
  */
 class CreatePassword extends \Magento\Customer\Controller\AbstractAccount implements HttpGetActionInterface
 {
     /**
-     * @var \Magento\Customer\Api\AccountManagementInterface
+     * @var AccountManagementInterface
      */
     protected $accountManagement;
 
@@ -38,37 +39,46 @@ class CreatePassword extends \Magento\Customer\Controller\AbstractAccount implem
     protected $resultPageFactory;
 
     /**
-     * @var \Magento\Customer\Model\ForgotPasswordToken\ConfirmCustomerByToken
+     * @var ConfirmCustomerByToken
      */
     private $confirmByToken;
 
     /**
-     * @param \Magento\Framework\App\Action\Context $context
-     * @param \Magento\Customer\Model\Session $customerSession
-     * @param \Magento\Framework\View\Result\PageFactory $resultPageFactory
-     * @param \Magento\Customer\Api\AccountManagementInterface $accountManagement
-     * @param \Magento\Customer\Model\ForgotPasswordToken\ConfirmCustomerByToken $confirmByToken
+     * @var GetCustomerByToken
+     */
+    private $getByToken;
+
+    /**
+     * @param Context $context
+     * @param Session $customerSession
+     * @param PageFactory $resultPageFactory
+     * @param AccountManagementInterface $accountManagement
+     * @param ConfirmCustomerByToken|null $confirmByToken
+     * @param GetCustomerByToken|null $getByToken
      */
     public function __construct(
         Context $context,
         Session $customerSession,
         PageFactory $resultPageFactory,
         AccountManagementInterface $accountManagement,
-        ConfirmCustomerByToken $confirmByToken = null
+        ConfirmCustomerByToken $confirmByToken = null,
+        GetCustomerByToken $getByToken = null
     ) {
         $this->session = $customerSession;
         $this->resultPageFactory = $resultPageFactory;
         $this->accountManagement = $accountManagement;
         $this->confirmByToken = $confirmByToken
             ?? ObjectManager::getInstance()->get(ConfirmCustomerByToken::class);
+        $this->getByToken = $getByToken
+            ?? ObjectManager::getInstance()->get(GetCustomerByToken::class);
 
         parent::__construct($context);
     }
 
     /**
      * Resetting password handler
      *
-     * @return \Magento\Framework\Controller\Result\Redirect|\Magento\Framework\View\Result\Page
+     * @return Redirect|Page
      */
     public function execute()
     {
@@ -83,14 +93,19 @@ public function execute()
 
             $this->confirmByToken->execute($resetPasswordToken);
 
+            // Extend token validity to avoid expiration while this form is
+            // being completed by the user.
+            $customer = $this->getByToken->execute($resetPasswordToken);
+            $this->accountManagement->changeResetPasswordLinkToken($customer, $resetPasswordToken);
+
             if ($isDirectLink) {
                 $this->session->setRpToken($resetPasswordToken);
                 $resultRedirect = $this->resultRedirectFactory->create();
                 $resultRedirect->setPath('*/*/createpassword');
 
                 return $resultRedirect;
             } else {
-                /** @var \Magento\Framework\View\Result\Page $resultPage */
+                /** @var Page $resultPage */
                 $resultPage = $this->resultPageFactory->create();
                 $resultPage->getLayout()
                            ->getBlock('resetPassword')
@@ -100,7 +115,7 @@ public function execute()
             }
         } catch (\Exception $exception) {
             $this->messageManager->addErrorMessage(__('Your password reset link has expired.'));
-            /** @var \Magento\Framework\Controller\Result\Redirect $resultRedirect */
+            /** @var Redirect $resultRedirect */
             $resultRedirect = $this->resultRedirectFactory->create();
             $resultRedirect->setPath('*/*/forgotpassword');
 
diff --git a/app/code/Magento/User/Controller/Adminhtml/Auth/ResetPassword.php b/app/code/Magento/User/Controller/Adminhtml/Auth/ResetPassword.php
@@ -1,12 +1,16 @@
 <?php
 /**
- *
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
 namespace Magento\User\Controller\Adminhtml\Auth;
 
-class ResetPassword extends \Magento\User\Controller\Adminhtml\Auth
+use Magento\Framework\App\Action\HttpGetActionInterface;
+
+/**
+ * Controller for admin user password reset form
+ */
+class ResetPassword extends \Magento\User\Controller\Adminhtml\Auth implements HttpGetActionInterface
 {
     /**
      * Display reset forgotten password form
@@ -22,6 +26,12 @@ public function execute()
         try {
             $this->_validateResetPasswordLinkToken($userId, $passwordResetToken);
 
+            // Extend token validity to avoid expiration while this form is
+            // being completed by the user.
+            $user = $this->_userFactory->create()->load($userId);
+            $user->changeResetPasswordLinkToken($passwordResetToken);
+            $user->save();
+
             $this->_view->loadLayout();
 
             $content = $this->_view->getLayout()->getBlock('content');
diff --git a/dev/tests/integration/testsuite/Magento/Customer/Controller/CreatePasswordTest.php b/dev/tests/integration/testsuite/Magento/Customer/Controller/CreatePasswordTest.php
@@ -10,8 +10,11 @@
 use Magento\Customer\Model\CustomerRegistry;
 use Magento\Customer\Model\ResourceModel\Customer as CustomerResource;
 use Magento\Customer\Model\Session;
+use Magento\Framework\Intl\DateTimeFactory;
 use Magento\Framework\Math\Random;
+use Magento\Framework\Message\MessageInterface;
 use Magento\Framework\ObjectManagerInterface;
+use Magento\Framework\Stdlib\DateTime;
 use Magento\Framework\View\LayoutInterface;
 use Magento\Store\Api\WebsiteRepositoryInterface;
 use Magento\TestFramework\Helper\Bootstrap;
@@ -42,6 +45,9 @@ class CreatePasswordTest extends AbstractController
     /** @var CustomerRegistry */
     private $customerRegistry;
 
+    /** @var DateTimeFactory */
+    private $dateTimeFactory;
+
     /** @var WebsiteRepositoryInterface */
     private $websiteRepository;
 
@@ -61,6 +67,7 @@ protected function setUp(): void
         $this->random = $this->objectManager->get(Random::class);
         $this->customerResource = $this->objectManager->get(CustomerResource::class);
         $this->customerRegistry = $this->objectManager->get(CustomerRegistry::class);
+        $this->dateTimeFactory = $this->objectManager->get(DateTimeFactory::class);
         $this->websiteRepository = $this->objectManager->get(WebsiteRepositoryInterface::class);
     }
 
@@ -94,4 +101,69 @@ public function testCreatePassword(): void
         $block = $this->layout->getBlock('resetPassword');
         $this->assertEquals($token, $block->getResetPasswordLinkToken());
     }
+
+    /**
+     * @magentoDataFixture Magento/Customer/_files/customer_with_website.php
+     *
+     * @return void
+     */
+    public function testTokenHasExpired(): void
+    {
+        $defaultWebsite = $this->websiteRepository->get('base')->getId();
+        $customer = $this->customerRegistry->retrieveByEmail('john.doe@magento.com', $defaultWebsite);
+        $this->customerId = $customer->getId();
+        $token = $this->random->getUniqueHash();
+        $tooLongAgo = $this->dateTimeFactory->create()
+            ->sub(\DateInterval::createFromDateString('1 month'))
+            ->format(DateTime::DATETIME_PHP_FORMAT);
+
+        $customer->changeResetPasswordLinkToken($token);
+        $customer->setData('confirmation', 'confirmation');
+        $customerSecure = $this->customerRegistry->retrieveSecureData($this->customerId);
+        $customerSecure->setRpTokenCreatedAt($tooLongAgo);
+        $this->customerResource->save($customer);
+
+        $this->session->setRpToken($token);
+        $this->session->setRpCustomerId($this->customerId);
+
+        $this->dispatch('customer/account/createPassword');
+
+        $this->assertRedirect($this->stringContains('customer/account/forgotpassword'));
+        $this->assertSessionMessages(
+            $this->equalTo(['Your password reset link has expired.']),
+            MessageInterface::TYPE_ERROR
+        );
+    }
+
+    /**
+     * @magentoDataFixture Magento/Customer/_files/customer_with_website.php
+     *
+     * @return void
+     */
+    public function testTokenExtendedOnPageLoad(): void
+    {
+        $defaultWebsite = $this->websiteRepository->get('base')->getId();
+        $customer = $this->customerRegistry->retrieveByEmail('john.doe@magento.com', $defaultWebsite);
+        $this->customerId = $customer->getId();
+        $token = $this->random->getUniqueHash();
+        $anHourAgo = $this->dateTimeFactory->create()
+            ->sub(\DateInterval::createFromDateString('1 hour'))
+            ->format(DateTime::DATETIME_PHP_FORMAT);
+
+        $customer->changeResetPasswordLinkToken($token);
+        $customer->setData('confirmation', 'confirmation');
+        $customerSecure = $this->customerRegistry->retrieveSecureData($this->customerId);
+        $customerSecure->setRpTokenCreatedAt($anHourAgo);
+        $this->customerResource->save($customer);
+
+        $this->session->setRpToken($token);
+        $this->session->setRpCustomerId($this->customerId);
+
+        $this->dispatch('customer/account/createPassword');
+        $block = $this->layout->getBlock('resetPassword');
+        $this->assertEquals($token, $block->getResetPasswordLinkToken());
+
+        $customerSecure = $this->customerRegistry->retrieveSecureData($this->customerId);
+        $this->assertNotEquals($anHourAgo, $customerSecure->getRpTokenCreatedAt());
+    }
 }
diff --git a/dev/tests/integration/testsuite/Magento/User/Controller/Adminhtml/AuthTest.php b/dev/tests/integration/testsuite/Magento/User/Controller/Adminhtml/AuthTest.php
@@ -5,13 +5,16 @@
  */
 namespace Magento\User\Controller\Adminhtml;
 
+use Magento\Framework\Intl\DateTimeFactory;
+use Magento\Framework\Stdlib\DateTime;
 use Magento\TestFramework\Mail\Template\TransportBuilderMock;
 use Magento\TestFramework\Helper\Bootstrap;
 
 /**
  * Test class for \Magento\User\Controller\Adminhtml\Auth
  *
  * @magentoAppArea adminhtml
+ * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
 class AuthTest extends \Magento\TestFramework\TestCase\AbstractBackendController
 {
@@ -106,6 +109,45 @@ public function testResetPasswordAction()
         $this->assertTrue((bool)strpos($this->getResponse()->getBody(), $resetPasswordToken));
     }
 
+    /**
+     * Test reset password action extends expiry of token
+     *
+     * @covers \Magento\User\Controller\Adminhtml\Auth\ResetPassword::execute
+     * @covers \Magento\User\Controller\Adminhtml\Auth\ResetPassword::_validateResetPasswordLinkToken
+     * @magentoDataFixture Magento/User/_files/dummy_user.php
+     */
+    public function testResetPasswordActionWithTokenNearExpiry()
+    {
+        /** @var $user \Magento\User\Model\User */
+        $user = Bootstrap::getObjectManager()->create(
+            \Magento\User\Model\User::class
+        )->loadByUsername(
+            'dummy_username'
+        );
+        $this->assertNotEmpty($user->getId(), 'Broken fixture');
+        $resetPasswordToken = Bootstrap::getObjectManager()->get(
+            \Magento\User\Helper\Data::class
+        )->generateResetPasswordLinkToken();
+        $user->changeResetPasswordLinkToken($resetPasswordToken);
+
+        $anHourAgo = Bootstrap::getObjectManager()->create(DateTimeFactory::class)
+            ->create()
+            ->sub(\DateInterval::createFromDateString('1 hour'))
+            ->format(DateTime::DATETIME_PHP_FORMAT);
+        $user->setRpTokenCreatedAt($anHourAgo);
+        $user->save();
+
+        $this->getRequest()->setQueryValue('token', $resetPasswordToken)->setQueryValue('id', $user->getId());
+        $this->dispatch('backend/admin/auth/resetpassword');
+
+        $this->assertEquals('adminhtml', $this->getRequest()->getRouteName());
+        $this->assertEquals('auth', $this->getRequest()->getControllerName());
+        $this->assertEquals('resetpassword', $this->getRequest()->getActionName());
+        $this->assertTrue((bool)strpos($this->getResponse()->getBody(), $resetPasswordToken));
+
+        $this->assertNotEquals($anHourAgo, $user->reload()->getRpTokenCreatedAt());
+    }
+
     /**
      * @covers \Magento\User\Controller\Adminhtml\Auth\ResetPassword::execute
      * @covers \Magento\User\Controller\Adminhtml\Auth\ResetPassword::_validateResetPasswordLinkToken
