Merge remote-tracking branch 'origin/MC-21666' into 2.3.5-develop-pr109

Author: zakdma

app/code/Magento/Customer/Api/SessionCleanerInterface.php

@@ -0,0 +1,22 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types=1);
+
+namespace Magento\Customer\Api;
+
+/**
+ * Interface for cleaning customer session data.
+ */
+interface SessionCleanerInterface
+{
+    /**
+     * Destroy all active customer sessions related to given customer id, including current session.
+     *
+     * @param int $customerId
+     * @return void
+     */
+    public function clearFor(int $customerId): void;
+}

app/code/Magento/Customer/Model/AccountManagement.php

@@ -13,6 +13,7 @@
 use Magento\Customer\Api\Data\AddressInterface;
 use Magento\Customer\Api\Data\CustomerInterface;
 use Magento\Customer\Api\Data\ValidationResultsInterfaceFactory;
+use Magento\Customer\Api\SessionCleanerInterface;
 use Magento\Customer\Helper\View as CustomerViewHelper;
 use Magento\Customer\Model\Config\Share as ConfigShare;
 use Magento\Customer\Model\Customer as CustomerModel;
@@ -200,6 +201,7 @@ class AccountManagement implements AccountManagementInterface
      * Minimum password length
      *
      * @deprecated Get rid of Helpers in Password Security Management
+     * @see \Magento\Customer\Model\AccountManagement::XML_PATH_MINIMUM_PASSWORD_LENGTH
      */
     const MIN_PASSWORD_LENGTH = 6;
 
@@ -283,21 +285,6 @@ class AccountManagement implements AccountManagementInterface
      */
     private $transportBuilder;
 
-    /**
-     * @var SessionManagerInterface
-     */
-    private $sessionManager;
-
-    /**
-     * @var SaveHandlerInterface
-     */
-    private $saveHandler;
-
-    /**
-     * @var CollectionFactory
-     */
-    private $visitorCollectionFactory;
-
     /**
      * @var DataObjectProcessor
      */
@@ -383,6 +370,11 @@ class AccountManagement implements AccountManagementInterface
      */
     private $getByToken;
 
+    /**
+     * @var SessionCleanerInterface
+     */
+    private $sessionCleaner;
+
     /**
      * @param CustomerFactory $customerFactory
      * @param ManagerInterface $eventManager
@@ -417,10 +409,12 @@ class AccountManagement implements AccountManagementInterface
      * @param AddressRegistry|null $addressRegistry
      * @param GetCustomerByToken|null $getByToken
      * @param AllowedCountries|null $allowedCountriesReader
+     * @param SessionCleanerInterface|null $sessionCleaner
      * @SuppressWarnings(PHPMD.CyclomaticComplexity)
      * @SuppressWarnings(PHPMD.ExcessiveParameterList)
      * @SuppressWarnings(PHPMD.NPathComplexity)
      * @SuppressWarnings(PHPMD.LongVariable)
+     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
      */
     public function __construct(
         CustomerFactory $customerFactory,
@@ -455,7 +449,8 @@ public function __construct(
         SearchCriteriaBuilder $searchCriteriaBuilder = null,
         AddressRegistry $addressRegistry = null,
         GetCustomerByToken $getByToken = null,
-        AllowedCountries $allowedCountriesReader = null
+        AllowedCountries $allowedCountriesReader = null,
+        SessionCleanerInterface $sessionCleaner = null
     ) {
         $this->customerFactory = $customerFactory;
         $this->eventManager = $eventManager;
@@ -486,12 +481,6 @@ public function __construct(
         $this->dateTimeFactory = $dateTimeFactory ?: $objectManager->get(DateTimeFactory::class);
         $this->accountConfirmation = $accountConfirmation ?: $objectManager
             ->get(AccountConfirmation::class);
-        $this->sessionManager = $sessionManager
-            ?: $objectManager->get(SessionManagerInterface::class);
-        $this->saveHandler = $saveHandler
-            ?: $objectManager->get(SaveHandlerInterface::class);
-        $this->visitorCollectionFactory = $visitorCollectionFactory
-            ?: $objectManager->get(CollectionFactory::class);
         $this->searchCriteriaBuilder = $searchCriteriaBuilder
             ?: $objectManager->get(SearchCriteriaBuilder::class);
         $this->addressRegistry = $addressRegistry
@@ -500,6 +489,7 @@ public function __construct(
             ?: $objectManager->get(GetCustomerByToken::class);
         $this->allowedCountriesReader = $allowedCountriesReader
             ?: $objectManager->get(AllowedCountries::class);
+        $this->sessionCleaner = $sessionCleaner ?? $objectManager->get(SessionCleanerInterface::class);
     }
 
     /**
@@ -538,6 +528,8 @@ public function resendConfirmation($email, $websiteId = null, $redirectUrl = '')
         } catch (MailException $e) {
             // If we are not able to send a new account email, this should be ignored
             $this->logger->critical($e);
+
+            return false;
         }
         return true;
     }
@@ -725,7 +717,7 @@ public function resetPassword($email, $resetToken, $newPassword)
         $customerSecure->setRpToken(null);
         $customerSecure->setRpTokenCreatedAt(null);
         $customerSecure->setPasswordHash($this->createPasswordHash($newPassword));
-        $this->destroyCustomerSessions($customer->getId());
+        $this->sessionCleaner->clearFor((int)$customer->getId());
         $this->customerRepository->save($customer);
 
         return true;
@@ -1054,7 +1046,7 @@ private function changePasswordForCustomer($customer, $currentPassword, $newPass
         $customerSecure->setRpTokenCreatedAt(null);
         $this->checkPasswordStrength($newPassword);
         $customerSecure->setPasswordHash($this->createPasswordHash($newPassword));
-        $this->destroyCustomerSessions($customer->getId());
+        $this->sessionCleaner->clearFor((int)$customer->getId());
         $this->disableAddressValidation($customer);
         $this->customerRepository->save($customer);
 
@@ -1607,36 +1599,6 @@ private function getEmailNotification()
         }
     }
 
-    /**
-     * Destroy all active customer sessions by customer id (current session will not be destroyed).
-     *
-     * Customer sessions which should be deleted are collecting from the "customer_visitor" table considering
-     * configured session lifetime.
-     *
-     * @param string|int $customerId
-     * @return void
-     */
-    private function destroyCustomerSessions($customerId)
-    {
-        $sessionLifetime = $this->scopeConfig->getValue(
-            \Magento\Framework\Session\Config::XML_PATH_COOKIE_LIFETIME,
-            \Magento\Store\Model\ScopeInterface::SCOPE_STORE
-        );
-        $dateTime = $this->dateTimeFactory->create();
-        $activeSessionsTime = $dateTime->setTimestamp($dateTime->getTimestamp() - $sessionLifetime)
-            ->format(DateTime::DATETIME_PHP_FORMAT);
-        /** @var \Magento\Customer\Model\ResourceModel\Visitor\Collection $visitorCollection */
-        $visitorCollection = $this->visitorCollectionFactory->create();
-        $visitorCollection->addFieldToFilter('customer_id', $customerId);
-        $visitorCollection->addFieldToFilter('last_visit_at', ['from' => $activeSessionsTime]);
-        $visitorCollection->addFieldToFilter('session_id', ['neq' => $this->sessionManager->getSessionId()]);
-        /** @var \Magento\Customer\Model\Visitor $visitor */
-        foreach ($visitorCollection->getItems() as $visitor) {
-            $sessionId = $visitor->getSessionId();
-            $this->saveHandler->destroy($sessionId);
-        }
-    }
-
     /**
      * Set ignore_validation_flag for reset password flow to skip unnecessary address and customer validation
      *

app/code/Magento/Customer/Model/Session/SessionCleaner.php

@@ -0,0 +1,100 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types=1);
+
+namespace Magento\Customer\Model\Session;
+
+use Magento\Customer\Api\SessionCleanerInterface;
+use Magento\Customer\Model\ResourceModel\Visitor\CollectionFactory as VisitorCollectionFactory;
+use Magento\Framework\App\Config\ScopeConfigInterface;
+use Magento\Framework\Intl\DateTimeFactory;
+use Magento\Framework\Session\Config;
+use Magento\Framework\Session\SaveHandlerInterface;
+use Magento\Framework\Session\SessionManagerInterface;
+use Magento\Framework\Stdlib\DateTime;
+use Magento\Store\Model\ScopeInterface;
+
+/**
+ * Deletes all session data which relates to customer, including current session data.
+ *
+ * @SuppressWarnings(PHPMD.CookieAndSessionMisuse)
+ */
+class SessionCleaner implements SessionCleanerInterface
+{
+    /**
+     * @var ScopeConfigInterface
+     */
+    private $scopeConfig;
+
+    /**
+     * @var DateTimeFactory
+     */
+    private $dateTimeFactory;
+
+    /**
+     * @var VisitorCollectionFactory
+     */
+    private $visitorCollectionFactory;
+
+    /**
+     * @var SessionManagerInterface
+     */
+    private $sessionManager;
+
+    /**
+     * @var SaveHandlerInterface
+     */
+    private $saveHandler;
+
+    /**
+     * @inheritdoc
+     */
+    public function __construct(
+        ScopeConfigInterface $scopeConfig,
+        DateTimeFactory $dateTimeFactory,
+        VisitorCollectionFactory $visitorCollectionFactory,
+        SessionManagerInterface $sessionManager,
+        SaveHandlerInterface $saveHandler
+    ) {
+        $this->scopeConfig = $scopeConfig;
+        $this->dateTimeFactory = $dateTimeFactory;
+        $this->visitorCollectionFactory = $visitorCollectionFactory;
+        $this->sessionManager = $sessionManager;
+        $this->saveHandler = $saveHandler;
+    }
+
+    /**
+     * @inheritdoc
+     */
+    public function clearFor(int $customerId): void
+    {
+        if ($this->sessionManager->isSessionExists()) {
+            //delete old session and move data to the new session
+            //use this instead of $this->sessionManager->regenerateId because last one doesn't delete old session
+            // phpcs:ignore Magento2.Functions.DiscouragedFunction
+            session_regenerate_id(true);
+        }
+
+        $sessionLifetime = $this->scopeConfig->getValue(
+            Config::XML_PATH_COOKIE_LIFETIME,
+            ScopeInterface::SCOPE_STORE
+        );
+        $dateTime = $this->dateTimeFactory->create();
+        $activeSessionsTime = $dateTime->setTimestamp($dateTime->getTimestamp() - $sessionLifetime)
+            ->format(DateTime::DATETIME_PHP_FORMAT);
+        /** @var \Magento\Customer\Model\ResourceModel\Visitor\Collection $visitorCollection */
+        $visitorCollection = $this->visitorCollectionFactory->create();
+        $visitorCollection->addFieldToFilter('customer_id', $customerId);
+        $visitorCollection->addFieldToFilter('last_visit_at', ['from' => $activeSessionsTime]);
+        /** @var \Magento\Customer\Model\Visitor $visitor */
+        foreach ($visitorCollection->getItems() as $visitor) {
+            $sessionId = $visitor->getSessionId();
+            $this->sessionManager->start();
+            $this->saveHandler->destroy($sessionId);
+            $this->sessionManager->writeClose();
+        }
+    }
+}

app/code/Magento/Customer/Model/Visitor.php

@@ -10,7 +10,7 @@
 use Magento\Framework\App\RequestSafetyInterface;
 
 /**
- * Class Visitor
+ * Class Visitor responsible for initializing visitor's.
  *
  *  Used to track sessions of the logged in customers
  *
@@ -195,7 +195,9 @@ public function initByRequest($observer)
     public function saveByRequest($observer)
     {
         // prevent saving Visitor for safe methods, e.g. GET request
-        if ($this->skipRequestLogging || $this->requestSafety->isSafeMethod() || $this->isModuleIgnored($observer)) {
+        if (($this->skipRequestLogging || $this->requestSafety->isSafeMethod() || $this->isModuleIgnored($observer))
+            && !$this->sessionIdHasChanged()
+        ) {
             return $this;
         }
 
@@ -212,6 +214,23 @@ public function saveByRequest($observer)
         return $this;
     }
 
+    /**
+     * Check if visitor session id was changed.
+     *
+     * @return bool
+     */
+    private function sessionIdHasChanged(): bool
+    {
+        $visitorData = $this->session->getVisitorData();
+        $hasChanged = false;
+
+        if (isset($visitorData['session_id'])) {
+            $hasChanged = $this->session->getSessionId() !== $visitorData['session_id'];
+        }
+
+        return $hasChanged;
+    }
+
     /**
      * Returns true if the module is required
      *

app/code/Magento/Customer/Test/Mftf/Test/StorefrontUpdateCustomerPasswordTest.xml

@@ -78,4 +78,17 @@
         <remove keyForRemoval="loginWithNewPassword"/>
         <remove keyForRemoval="seeMyEmail"/>
     </test>
-</tests>
+    <test name="StorefrontUpdateCustomerPasswordRedirectOnLoginPage" extends="StorefrontUpdateCustomerPasswordValidCurrentPasswordTest">
+        <annotations>
+            <title value="Update Customer Password on Storefront Redirect on Login Page"/>
+            <description value="Update Customer Password on Storefront Redirect on Login Page"/>
+            <testCaseId value="MC-22957"/>
+            <useCaseId value="MC-21666"/>
+        </annotations>
+        <remove keyForRemoval="logout"/>
+        <remove keyForRemoval="loginWithNewPassword"/>
+        <remove keyForRemoval="seeMyEmail"/>
+
+        <seeInCurrentUrl url="{{StorefrontCustomerSignInPage.url}}" stepKey="assertStorefrontCustomerLoginPage"/>
+    </test>
+</tests>