From aa1543eda87e7c699a32b5659c470682c8863da6 Mon Sep 17 00:00:00 2001 From: Pavel Bystritsky Date: Thu, 30 Apr 2020 16:23:51 +0300 Subject: [PATCH 1/2] magento2-login-as-customer/issues/29: Destroy impersonated customer sessions on admin logout. --- .../Cron/DeleteExpiredAuthenticationData.php | 49 ------------ .../DeleteExpiredAuthenticationData.php | 9 +-- .../GetAuthenticationDataBySecret.php | 4 +- ...php => IsLoginAsCustomerSessionActive.php} | 22 +++--- .../Magento/LoginAsCustomer/etc/crontab.xml | 16 ---- app/code/Magento/LoginAsCustomer/etc/di.xml | 1 + ...leteExpiredAuthenticationDataInterface.php | 5 +- ...sLoginAsCustomerSessionActiveInterface.php | 25 ++++++ .../FrontAddCommentOnOrderPlacementPlugin.php | 4 +- .../Controller/Adminhtml/Login/Login.php | 14 +++- .../Controller/Login/Index.php | 11 --- .../Plugin/AdminLogoutPlugin.php | 48 ++++++++++++ .../Plugin/InvalidateExpiredSessionPlugin.php | 77 +++++++++++++++++++ .../LoginAsCustomerUi/etc/adminhtml/di.xml | 3 + .../LoginAsCustomerUi/etc/frontend/di.xml | 4 + 15 files changed, 192 insertions(+), 100 deletions(-) delete mode 100644 app/code/Magento/LoginAsCustomer/Cron/DeleteExpiredAuthenticationData.php rename app/code/Magento/LoginAsCustomer/Model/ResourceModel/{DeleteAuthenticationDataBySecret.php => IsLoginAsCustomerSessionActive.php} (61%) delete mode 100644 app/code/Magento/LoginAsCustomer/etc/crontab.xml create mode 100644 app/code/Magento/LoginAsCustomerApi/Api/IsLoginAsCustomerSessionActiveInterface.php create mode 100644 app/code/Magento/LoginAsCustomerUi/Plugin/AdminLogoutPlugin.php create mode 100644 app/code/Magento/LoginAsCustomerUi/Plugin/InvalidateExpiredSessionPlugin.php diff --git a/app/code/Magento/LoginAsCustomer/Cron/DeleteExpiredAuthenticationData.php b/app/code/Magento/LoginAsCustomer/Cron/DeleteExpiredAuthenticationData.php deleted file mode 100644 index 5acb602491f0b..0000000000000 --- a/app/code/Magento/LoginAsCustomer/Cron/DeleteExpiredAuthenticationData.php +++ /dev/null @@ -1,49 +0,0 @@ -deleteOldSecretsProcessor = $deleteOldSecretsProcessor; - $this->config = $config; - } - - /** - * Delete expired authentication data - */ - public function execute(): void - { - if ($this->config->isEnabled()) { - $this->deleteOldSecretsProcessor->execute(); - } - } -} diff --git a/app/code/Magento/LoginAsCustomer/Model/ResourceModel/DeleteExpiredAuthenticationData.php b/app/code/Magento/LoginAsCustomer/Model/ResourceModel/DeleteExpiredAuthenticationData.php index d838ba88b7d50..8778c20ab9257 100644 --- 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 index 7580668358b5a..8951ae8f70939 100644 --- 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/DeleteAuthenticationDataBySecret.php b/app/code/Magento/LoginAsCustomer/Model/ResourceModel/IsLoginAsCustomerSessionActive.php similarity index 61% rename from app/code/Magento/LoginAsCustomer/Model/ResourceModel/DeleteAuthenticationDataBySecret.php rename to app/code/Magento/LoginAsCustomer/Model/ResourceModel/IsLoginAsCustomerSessionActive.php index 6c12d4458ef1e..3bbba4b5e6ec4 100644 --- a/app/code/Magento/LoginAsCustomer/Model/ResourceModel/DeleteAuthenticationDataBySecret.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 deleted file mode 100644 index eaeef8cf8d759..0000000000000 --- a/app/code/Magento/LoginAsCustomer/etc/crontab.xml +++ /dev/null @@ -1,16 +0,0 @@ - - - - - - 15 * * * * - - - diff --git a/app/code/Magento/LoginAsCustomer/etc/di.xml b/app/code/Magento/LoginAsCustomer/etc/di.xml index 764e2cbd792e7..76602534d31e8 100755 --- a/app/code/Magento/LoginAsCustomer/etc/di.xml +++ b/app/code/Magento/LoginAsCustomer/etc/di.xml @@ -13,4 +13,5 @@ + diff --git a/app/code/Magento/LoginAsCustomerApi/Api/DeleteExpiredAuthenticationDataInterface.php b/app/code/Magento/LoginAsCustomerApi/Api/DeleteExpiredAuthenticationDataInterface.php index 30783d216354b..3b2e756f40e83 100644 --- 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 new file mode 100644 index 0000000000000..30674375ed021 --- /dev/null +++ b/app/code/Magento/LoginAsCustomerApi/Api/IsLoginAsCustomerSessionActiveInterface.php @@ -0,0 +1,25 @@ +config = $config; $this->authenticationDataFactory = $authenticationDataFactory; $this->saveAuthenticationData = $saveAuthenticationData; + $this->deleteExpiredAuthenticationData = $deleteExpiredAuthenticationData; $this->url = $url; } @@ -142,15 +151,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 index 0532846cf2535..eb9e6e0497029 100755 --- 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 new file mode 100644 index 0000000000000..bf8be414c787a --- /dev/null +++ b/app/code/Magento/LoginAsCustomerUi/Plugin/AdminLogoutPlugin.php @@ -0,0 +1,48 @@ +config = $config; + $this->deleteExpiredAuthenticationData = $deleteExpiredAuthenticationData; + } + + /** + * @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 new file mode 100644 index 0000000000000..42c637e185197 --- /dev/null +++ b/app/code/Magento/LoginAsCustomerUi/Plugin/InvalidateExpiredSessionPlugin.php @@ -0,0 +1,77 @@ +session = $session; + $this->adminSessionInfoFactory = $adminSessionInfoFactory; + $this->isLoginAsCustomerSessionActive = $isLoginAsCustomerSessionActive; + $this->config = $config; + } + + /** + * Invalidate expired and not active Login as Customer sessions. + * + * @param ActionInterface $subject + * @throws \Magento\Framework\Exception\LocalizedException + * @throws \Magento\Framework\Exception\NoSuchEntityException + */ + public function beforeExecute(ActionInterface $subject) + { + if ($this->config->isEnabled()) { + $adminId = (int)$this->session->getLoggedAsCustomerAdmindId(); + $customerId = (int)$this->session->getCustomerId(); + if ($adminId && $customerId) { + if (!$this->isLoginAsCustomerSessionActive->execute($customerId, $adminId)) { + $this->session->destroy(); + } + } + } + } +} diff --git a/app/code/Magento/LoginAsCustomerUi/etc/adminhtml/di.xml b/app/code/Magento/LoginAsCustomerUi/etc/adminhtml/di.xml index ba85241397dd3..2ee0b83be573e 100644 --- a/app/code/Magento/LoginAsCustomerUi/etc/adminhtml/di.xml +++ b/app/code/Magento/LoginAsCustomerUi/etc/adminhtml/di.xml @@ -9,4 +9,7 @@ + + + diff --git a/app/code/Magento/LoginAsCustomerUi/etc/frontend/di.xml b/app/code/Magento/LoginAsCustomerUi/etc/frontend/di.xml index b102505de49f8..746e908234edc 100755 --- a/app/code/Magento/LoginAsCustomerUi/etc/frontend/di.xml +++ b/app/code/Magento/LoginAsCustomerUi/etc/frontend/di.xml @@ -13,4 +13,8 @@ + + + From 6d955c87267c107b32f018197cc6a1f00a3ea0eb Mon Sep 17 00:00:00 2001 From: Pavel Bystritsky Date: Thu, 30 Apr 2020 19:59:36 +0300 Subject: [PATCH 2/2] magento2-login-as-customer/issues/29: Static tests fix. --- .../Controller/Adminhtml/Login/Login.php | 2 ++ .../Plugin/AdminLogoutPlugin.php | 13 +++++++++---- .../Plugin/InvalidateExpiredSessionPlugin.php | 18 +++++------------- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/app/code/Magento/LoginAsCustomerUi/Controller/Adminhtml/Login/Login.php b/app/code/Magento/LoginAsCustomerUi/Controller/Adminhtml/Login/Login.php index 3018007e39699..2235791029de5 100755 --- a/app/code/Magento/LoginAsCustomerUi/Controller/Adminhtml/Login/Login.php +++ b/app/code/Magento/LoginAsCustomerUi/Controller/Adminhtml/Login/Login.php @@ -31,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 { diff --git a/app/code/Magento/LoginAsCustomerUi/Plugin/AdminLogoutPlugin.php b/app/code/Magento/LoginAsCustomerUi/Plugin/AdminLogoutPlugin.php index bf8be414c787a..ff61b9482ac17 100644 --- a/app/code/Magento/LoginAsCustomerUi/Plugin/AdminLogoutPlugin.php +++ b/app/code/Magento/LoginAsCustomerUi/Plugin/AdminLogoutPlugin.php @@ -8,13 +8,16 @@ namespace Magento\LoginAsCustomerUi\Plugin; use Magento\Backend\Model\Auth; -use Magento\LoginAsCustomer\Model\Config; +use Magento\LoginAsCustomerApi\Api\ConfigInterface; use Magento\LoginAsCustomerApi\Api\DeleteExpiredAuthenticationDataInterface; +/** + * Delete all Login as Customer sessions for logging out admin. + */ class AdminLogoutPlugin { /** - * @var Config + * @var ConfigInterface */ private $config; @@ -24,11 +27,11 @@ class AdminLogoutPlugin private $deleteExpiredAuthenticationData; /** - * @param Config $config + * @param ConfigInterface $config * @param DeleteExpiredAuthenticationDataInterface $deleteExpiredAuthenticationData */ public function __construct( - Config $config, + ConfigInterface $config, DeleteExpiredAuthenticationDataInterface $deleteExpiredAuthenticationData ) { $this->config = $config; @@ -36,6 +39,8 @@ public function __construct( } /** + * Delete all Login as Customer sessions for logging out admin. + * * @param Auth $subject */ public function beforeLogout(Auth $subject): void diff --git a/app/code/Magento/LoginAsCustomerUi/Plugin/InvalidateExpiredSessionPlugin.php b/app/code/Magento/LoginAsCustomerUi/Plugin/InvalidateExpiredSessionPlugin.php index 42c637e185197..1f80292ed2738 100644 --- a/app/code/Magento/LoginAsCustomerUi/Plugin/InvalidateExpiredSessionPlugin.php +++ b/app/code/Magento/LoginAsCustomerUi/Plugin/InvalidateExpiredSessionPlugin.php @@ -8,9 +8,8 @@ use Magento\Customer\Model\Session; use Magento\Framework\App\ActionInterface; -use Magento\LoginAsCustomer\Model\Config; +use Magento\LoginAsCustomerApi\Api\ConfigInterface; use Magento\LoginAsCustomerApi\Api\IsLoginAsCustomerSessionActiveInterface; -use Magento\Security\Model\AdminSessionInfoFactory; /** * Invalidate expired and not active Login as Customer sessions. @@ -18,7 +17,7 @@ class InvalidateExpiredSessionPlugin { /** - * @var Config + * @var ConfigInterface */ private $config; @@ -33,24 +32,16 @@ class InvalidateExpiredSessionPlugin private $isLoginAsCustomerSessionActive; /** - * @var AdminSessionInfoFactory - */ - private $adminSessionInfoFactory; - - /** - * @param Config $config + * @param ConfigInterface $config * @param Session $session - * @param AdminSessionInfoFactory $adminSessionInfoFactory * @param IsLoginAsCustomerSessionActiveInterface $isLoginAsCustomerSessionActive */ public function __construct( - Config $config, + ConfigInterface $config, Session $session, - AdminSessionInfoFactory $adminSessionInfoFactory, IsLoginAsCustomerSessionActiveInterface $isLoginAsCustomerSessionActive ) { $this->session = $session; - $this->adminSessionInfoFactory = $adminSessionInfoFactory; $this->isLoginAsCustomerSessionActive = $isLoginAsCustomerSessionActive; $this->config = $config; } @@ -61,6 +52,7 @@ public function __construct( * @param ActionInterface $subject * @throws \Magento\Framework\Exception\LocalizedException * @throws \Magento\Framework\Exception\NoSuchEntityException + * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ public function beforeExecute(ActionInterface $subject) {