Merge pull request #52 from magento-commerce/magento2-login-as-customer/issues/29

naydav · web-flow · commit adb8f4d7b3d7 · 2020-04-30T14:13:54.000-05:00
magento2-login-as-customer/issues/29: Destroy impersonated customer sessions on admin logout.
diff --git a/app/code/Magento/LoginAsCustomer/Cron/DeleteExpiredAuthenticationData.php b/app/code/Magento/LoginAsCustomer/Cron/DeleteExpiredAuthenticationData.php
diff --git a/app/code/Magento/LoginAsCustomer/Model/ResourceModel/DeleteExpiredAuthenticationData.php b/app/code/Magento/LoginAsCustomer/Model/ResourceModel/DeleteExpiredAuthenticationData.php
@@ -50,20 +50,15 @@ public function __construct(
     /**
      * @inheritdoc
      */
-    public function execute(): void
+    public function execute(int $userId): void
     {
         $connection = $this->resourceConnection->getConnection();
         $tableName = $this->resourceConnection->getTableName('login_as_customer');
 
-        $timePoint = date(
-            'Y-m-d H:i:s',
-            $this->dateTime->gmtTimestamp() - $this->config->getAuthenticationDataExpirationTime()
-        );
-
         $connection->delete(
             $tableName,
             [
-                'created_at < ?' => $timePoint
+                'admin_id = ?' => $userId
             ]
         );
     }
diff --git a/app/code/Magento/LoginAsCustomer/Model/ResourceModel/GetAuthenticationDataBySecret.php b/app/code/Magento/LoginAsCustomer/Model/ResourceModel/GetAuthenticationDataBySecret.php
@@ -85,8 +85,8 @@ public function execute(string $secretKey): AuthenticationDataInterface
         /** @var AuthenticationDataInterface $authenticationData */
         $authenticationData = $this->authenticationDataFactory->create(
             [
-                'customerId' => (int)$data['admin_id'],
-                'adminId' => (int)$data['customer_id'],
+                'customerId' => (int)$data['customer_id'],
+                'adminId' => (int)$data['admin_id'],
                 'extensionAttributes' => null,
             ]
         );
diff --git a/app/code/Magento/LoginAsCustomer/Model/ResourceModel/IsLoginAsCustomerSessionActive.php b/app/code/Magento/LoginAsCustomer/Model/ResourceModel/IsLoginAsCustomerSessionActive.php
@@ -8,12 +8,12 @@
 namespace Magento\LoginAsCustomer\Model\ResourceModel;
 
 use Magento\Framework\App\ResourceConnection;
-use Magento\LoginAsCustomerApi\Api\DeleteAuthenticationDataBySecretInterface;
+use Magento\LoginAsCustomerApi\Api\IsLoginAsCustomerSessionActiveInterface;
 
 /**
  * @inheritdoc
  */
-class DeleteAuthenticationDataBySecret implements DeleteAuthenticationDataBySecretInterface
+class IsLoginAsCustomerSessionActive implements IsLoginAsCustomerSessionActiveInterface
 {
     /**
      * @var ResourceConnection
@@ -32,16 +32,18 @@ public function __construct(
     /**
      * @inheritdoc
      */
-    public function execute(string $secret): void
+    public function execute(int $customerId, int $userId): bool
     {
-        $connection = $this->resourceConnection->getConnection();
         $tableName = $this->resourceConnection->getTableName('login_as_customer');
+        $connection = $this->resourceConnection->getConnection();
+
+        $query = $connection->select()
+            ->from($tableName)
+            ->where('customer_id = ?', $customerId)
+            ->where('admin_id = ?', $userId);
+
+        $result = $connection->fetchRow($query);
 
-        $connection->delete(
-            $tableName,
-            [
-                'secret = ?' => $secret
-            ]
-        );
+        return false !== $result;
     }
 }
diff --git a/app/code/Magento/LoginAsCustomer/etc/crontab.xml b/app/code/Magento/LoginAsCustomer/etc/crontab.xml
diff --git a/app/code/Magento/LoginAsCustomer/etc/di.xml b/app/code/Magento/LoginAsCustomer/etc/di.xml
@@ -13,4 +13,5 @@
     <preference for="Magento\LoginAsCustomerApi\Api\DeleteAuthenticationDataBySecretInterface" type="Magento\LoginAsCustomer\Model\ResourceModel\DeleteAuthenticationDataBySecret"/>
     <preference for="Magento\LoginAsCustomerApi\Api\DeleteExpiredAuthenticationDataInterface" type="Magento\LoginAsCustomer\Model\ResourceModel\DeleteExpiredAuthenticationData"/>
     <preference for="Magento\LoginAsCustomerApi\Api\ConfigInterface" type="Magento\LoginAsCustomer\Model\Config"/>
+    <preference for="Magento\LoginAsCustomerApi\Api\IsLoginAsCustomerSessionActiveInterface" type="Magento\LoginAsCustomer\Model\ResourceModel\IsLoginAsCustomerSessionActive"/>
 </config>
diff --git a/app/code/Magento/LoginAsCustomerApi/Api/DeleteExpiredAuthenticationDataInterface.php b/app/code/Magento/LoginAsCustomerApi/Api/DeleteExpiredAuthenticationDataInterface.php
@@ -15,9 +15,10 @@
 interface DeleteExpiredAuthenticationDataInterface
 {
     /**
-     * Delete expired authentication data
+     * Delete expired authentication data by user id.
      *
+     * @param int $userId
      * @return void
      */
-    public function execute(): void;
+    public function execute(int $userId): void;
 }
diff --git a/app/code/Magento/LoginAsCustomerApi/Api/IsLoginAsCustomerSessionActiveInterface.php b/app/code/Magento/LoginAsCustomerApi/Api/IsLoginAsCustomerSessionActiveInterface.php
@@ -0,0 +1,25 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types=1);
+
+namespace Magento\LoginAsCustomerApi\Api;
+
+/**
+ * Check if Login as Customer session is still active.
+ *
+ * @api
+ */
+interface IsLoginAsCustomerSessionActiveInterface
+{
+    /**
+     * Check if Login as Customer session is still active.
+     *
+     * @param int $customerId
+     * @param int $userId
+     * @return bool
+     */
+    public function execute(int $customerId, int $userId): bool;
+}
diff --git a/app/code/Magento/LoginAsCustomerSales/Plugin/FrontAddCommentOnOrderPlacementPlugin.php b/app/code/Magento/LoginAsCustomerSales/Plugin/FrontAddCommentOnOrderPlacementPlugin.php
@@ -12,7 +12,7 @@
 use Magento\User\Model\UserFactory;
 
 /**
- * Add comment after order placed by admin using login-as-customer.
+ * Add comment after order placed by admin using Login as Customer.
  *
  * @SuppressWarnings(PHPMD.CookieAndSessionMisuse)
  */
@@ -41,7 +41,7 @@ public function __construct(
     }
 
     /**
-     * Add comment after order placed by admin using login-as-customer.
+     * Add comment after order placed by admin using Login as Customer.
      *
      * @param Order $subject
      * @param Order $result
diff --git a/app/code/Magento/LoginAsCustomerUi/Controller/Adminhtml/Login/Login.php b/app/code/Magento/LoginAsCustomerUi/Controller/Adminhtml/Login/Login.php
@@ -22,6 +22,7 @@
 use Magento\LoginAsCustomerApi\Api\ConfigInterface;
 use Magento\LoginAsCustomerApi\Api\Data\AuthenticationDataInterface;
 use Magento\LoginAsCustomerApi\Api\Data\AuthenticationDataInterfaceFactory;
+use Magento\LoginAsCustomerApi\Api\DeleteExpiredAuthenticationDataInterface;
 use Magento\LoginAsCustomerApi\Api\SaveAuthenticationDataInterface;
 use Magento\Store\Model\StoreManagerInterface;
 
@@ -30,6 +31,8 @@
  * Generate secret key and forward to the storefront action
  *
  * This action can be executed via GET request when "Store View To Login In" is disabled, and POST when it is enabled
+ *
+ * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
 class Login extends Action implements HttpGetActionInterface, HttpPostActionInterface
 {
@@ -70,6 +73,11 @@ class Login extends Action implements HttpGetActionInterface, HttpPostActionInte
      */
     private $saveAuthenticationData;
 
+    /**
+     * @var DeleteExpiredAuthenticationDataInterface
+     */
+    private $deleteExpiredAuthenticationData;
+
     /**
      * @var Url
      */
@@ -83,6 +91,7 @@ class Login extends Action implements HttpGetActionInterface, HttpPostActionInte
      * @param ConfigInterface $config
      * @param AuthenticationDataInterfaceFactory $authenticationDataFactory
      * @param SaveAuthenticationDataInterface $saveAuthenticationData ,
+     * @param DeleteExpiredAuthenticationDataInterface $deleteExpiredAuthenticationData
      * @param Url $url
      */
     public function __construct(
@@ -93,6 +102,7 @@ public function __construct(
         ConfigInterface $config,
         AuthenticationDataInterfaceFactory $authenticationDataFactory,
         SaveAuthenticationDataInterface $saveAuthenticationData,
+        DeleteExpiredAuthenticationDataInterface $deleteExpiredAuthenticationData,
         Url $url
     ) {
         parent::__construct($context);
@@ -103,6 +113,7 @@ public function __construct(
         $this->config = $config;
         $this->authenticationDataFactory = $authenticationDataFactory;
         $this->saveAuthenticationData = $saveAuthenticationData;
+        $this->deleteExpiredAuthenticationData = $deleteExpiredAuthenticationData;
         $this->url = $url;
     }
 
@@ -142,15 +153,18 @@ public function execute(): ResultInterface
         }
 
         $adminUser = $this->authSession->getUser();
+        $userId = (int)$adminUser->getId();
 
         /** @var AuthenticationDataInterface $authenticationData */
         $authenticationData = $this->authenticationDataFactory->create(
             [
                 'customerId' => $customerId,
-                'adminId' => (int)$adminUser->getId(),
+                'adminId' => $userId,
                 'extensionAttributes' => null,
             ]
         );
+
+        $this->deleteExpiredAuthenticationData->execute($userId);
         $secret = $this->saveAuthenticationData->execute($authenticationData);
 
         $redirectUrl = $this->getLoginProceedRedirectUrl($secret, $storeId);
diff --git a/app/code/Magento/LoginAsCustomerUi/Controller/Login/Index.php b/app/code/Magento/LoginAsCustomerUi/Controller/Login/Index.php
@@ -18,7 +18,6 @@
 use Magento\Framework\Message\ManagerInterface;
 use Magento\LoginAsCustomerApi\Api\GetAuthenticationDataBySecretInterface;
 use Magento\LoginAsCustomerApi\Api\AuthenticateCustomerInterface;
-use Magento\LoginAsCustomerApi\Api\DeleteAuthenticationDataBySecretInterface;
 use Psr\Log\LoggerInterface;
 
 /**
@@ -51,11 +50,6 @@ class Index implements HttpGetActionInterface
      */
     private $authenticateCustomer;
 
-    /**
-     * @var DeleteAuthenticationDataBySecretInterface
-     */
-    private $deleteAuthenticationDataBySecret;
-
     /**
      * @var ManagerInterface
      */
@@ -72,7 +66,6 @@ class Index implements HttpGetActionInterface
      * @param CustomerRepositoryInterface $customerRepository
      * @param GetAuthenticationDataBySecretInterface $getAuthenticateDataProcessor
      * @param AuthenticateCustomerInterface $authenticateCustomerProcessor
-     * @param DeleteAuthenticationDataBySecretInterface $deleteSecretProcessor
      * @param ManagerInterface $messageManager
      * @param LoggerInterface $logger
      */
@@ -82,7 +75,6 @@ public function __construct(
         CustomerRepositoryInterface $customerRepository,
         GetAuthenticationDataBySecretInterface $getAuthenticateDataProcessor,
         AuthenticateCustomerInterface $authenticateCustomerProcessor,
-        DeleteAuthenticationDataBySecretInterface $deleteSecretProcessor,
         ManagerInterface $messageManager,
         LoggerInterface $logger
     ) {
@@ -91,7 +83,6 @@ public function __construct(
         $this->customerRepository = $customerRepository;
         $this->getAuthenticationDataBySecret = $getAuthenticateDataProcessor;
         $this->authenticateCustomer = $authenticateCustomerProcessor;
-        $this->deleteAuthenticationDataBySecret = $deleteSecretProcessor;
         $this->messageManager = $messageManager;
         $this->logger = $logger;
     }
@@ -114,8 +105,6 @@ public function execute(): ResultInterface
 
             $authenticateData = $this->getAuthenticationDataBySecret->execute($secret);
 
-            $this->deleteAuthenticationDataBySecret->execute($secret);
-
             try {
                 $customer = $this->customerRepository->getById($authenticateData->getCustomerId());
             } catch (NoSuchEntityException $e) {
diff --git a/app/code/Magento/LoginAsCustomerUi/Plugin/AdminLogoutPlugin.php b/app/code/Magento/LoginAsCustomerUi/Plugin/AdminLogoutPlugin.php
@@ -0,0 +1,53 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types=1);
+
+namespace Magento\LoginAsCustomerUi\Plugin;
+
+use Magento\Backend\Model\Auth;
+use Magento\LoginAsCustomerApi\Api\ConfigInterface;
+use Magento\LoginAsCustomerApi\Api\DeleteExpiredAuthenticationDataInterface;
+
+/**
+ * Delete all Login as Customer sessions for logging out admin.
+ */
+class AdminLogoutPlugin
+{
+    /**
+     * @var ConfigInterface
+     */
+    private $config;
+
+    /**
+     * @var DeleteExpiredAuthenticationDataInterface
+     */
+    private $deleteExpiredAuthenticationData;
+
+    /**
+     * @param ConfigInterface $config
+     * @param DeleteExpiredAuthenticationDataInterface $deleteExpiredAuthenticationData
+     */
+    public function __construct(
+        ConfigInterface $config,
+        DeleteExpiredAuthenticationDataInterface $deleteExpiredAuthenticationData
+    ) {
+        $this->config = $config;
+        $this->deleteExpiredAuthenticationData = $deleteExpiredAuthenticationData;
+    }
+
+    /**
+     * Delete all Login as Customer sessions for logging out admin.
+     *
+     * @param Auth $subject
+     */
+    public function beforeLogout(Auth $subject): void
+    {
+        if ($this->config->isEnabled()) {
+            $userId = (int)$subject->getUser()->getId();
+            $this->deleteExpiredAuthenticationData->execute($userId);
+        }
+    }
+}
diff --git a/app/code/Magento/LoginAsCustomerUi/Plugin/InvalidateExpiredSessionPlugin.php b/app/code/Magento/LoginAsCustomerUi/Plugin/InvalidateExpiredSessionPlugin.php
diff --git a/app/code/Magento/LoginAsCustomerUi/etc/adminhtml/di.xml b/app/code/Magento/LoginAsCustomerUi/etc/adminhtml/di.xml
diff --git a/app/code/Magento/LoginAsCustomerUi/etc/frontend/di.xml b/app/code/Magento/LoginAsCustomerUi/etc/frontend/di.xml

Original file line number	Diff line number	Diff line change
`@@ -50,20 +50,15 @@ public function __construct(`
`50`	`50`	`/**`
`51`	`51`	`* @inheritdoc`
`52`	`52`	`*/`
`53`		`- public function execute(): void`
	`53`	`+ public function execute(int $userId): void`
`54`	`54`	`{`
`55`	`55`	`$connection = $this->resourceConnection->getConnection();`
`56`	`56`	`$tableName = $this->resourceConnection->getTableName('login_as_customer');`
`57`	`57`
`58`		`- $timePoint = date(`
`59`		`- 'Y-m-d H:i:s',`
`60`		`- $this->dateTime->gmtTimestamp() - $this->config->getAuthenticationDataExpirationTime()`
`61`		`- );`
`62`		`-`
`63`	`58`	`$connection->delete(`
`64`	`59`	`$tableName,`
`65`	`60`	`[`
`66`		`- 'created_at < ?' => $timePoint`
	`61`	`+ 'admin_id = ?' => $userId`
`67`	`62`	`]`
`68`	`63`	`);`
`69`	`64`	`}`
Original file line number	Diff line number	Diff line change
`@@ -85,8 +85,8 @@ public function execute(string $secretKey): AuthenticationDataInterface`
`85`	`85`	`/** @var AuthenticationDataInterface $authenticationData */`
`86`	`86`	`$authenticationData = $this->authenticationDataFactory->create(`
`87`	`87`	`[`
`88`		`- 'customerId' => (int)$data['admin_id'],`
`89`		`- 'adminId' => (int)$data['customer_id'],`
	`88`	`+ 'customerId' => (int)$data['customer_id'],`
	`89`	`+ 'adminId' => (int)$data['admin_id'],`
`90`	`90`	`'extensionAttributes' => null,`
`91`	`91`	`]`
`92`	`92`	`);`
Original file line number	Diff line number	Diff line change
`@@ -15,9 +15,10 @@`
`15`	`15`	`interface DeleteExpiredAuthenticationDataInterface`
`16`	`16`	`{`
`17`	`17`	`/**`
`18`		`- * Delete expired authentication data`
	`18`	`+ * Delete expired authentication data by user id.`
`19`	`19`	`*`
	`20`	`+ * @param int $userId`
`20`	`21`	`* @return void`
`21`	`22`	`*/`
`22`		`- public function execute(): void;`
	`23`	`+ public function execute(int $userId): void;`
`23`	`24`	`}`
Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@`
`12`	`12`	`use Magento\User\Model\UserFactory;`
`13`	`13`
`14`	`14`	`/**`
`15`		`- * Add comment after order placed by admin using login-as-customer.`
	`15`	`+ * Add comment after order placed by admin using Login as Customer.`
`16`	`16`	`*`
`17`	`17`	`* @SuppressWarnings(PHPMD.CookieAndSessionMisuse)`
`18`	`18`	`*/`
`@@ -41,7 +41,7 @@ public function __construct(`
`41`	`41`	`}`
`42`	`42`
`43`	`43`	`/**`
`44`		`- * Add comment after order placed by admin using login-as-customer.`
	`44`	`+ * Add comment after order placed by admin using Login as Customer.`
`45`	`45`	`*`
`46`	`46`	`* @param Order $subject`
`47`	`47`	`* @param Order $result`