split out deactivateExpiredUsers into two methods suitable for callin…

…g without input parameters in cron (#22833)
magento · Oct 25, 2019 · 33a5d36 · 33a5d36
1 parent 4c4149f
commit 33a5d36
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 14 deletions.
diff --git a/app/code/Magento/Security/Model/Plugin/AuthSession.php b/app/code/Magento/Security/Model/Plugin/AuthSession.php
@@ -76,7 +76,7 @@ public function aroundProlong(Session $session, \Closure $proceed)
             $this->addUserLogoutNotification();
             return null;
         } elseif ($this->userExpirationManager->isUserExpired($session->getUser()->getId())) {
-            $this->userExpirationManager->deactivateExpiredUsers([$session->getUser()->getId()]);
+            $this->userExpirationManager->deactivateExpiredUsersById([$session->getUser()->getId()]);
             $session->destroy();
             $this->addUserLogoutNotification();
             return null;

diff --git a/app/code/Magento/Security/Model/UserExpirationManager.php b/app/code/Magento/Security/Model/UserExpirationManager.php
@@ -76,17 +76,34 @@ public function __construct(
 
     /**
      * Deactivate expired user accounts and invalidate their sessions.
-     *
-     * @param array|null $userIds
      */
-    public function deactivateExpiredUsers(?array $userIds = null): void
+    public function deactivateExpiredUsers(): void
     {
         /** @var ExpiredUsersCollection $expiredRecords */
         $expiredRecords = $this->userExpirationCollectionFactory->create()->addActiveExpiredUsersFilter();
-        if ($userIds != null) {
-            $expiredRecords->addUserIdsFilter($userIds);
-        }
+        $this->processExpiredUsers($expiredRecords);
+    }
+
+    /**
+     * Deactivate specific expired users.
+     *
+     * @param array $userIds
+     */
+    public function deactivateExpiredUsersById(array $userIds): void
+    {
+        $expiredRecords = $this->userExpirationCollectionFactory->create()
+            ->addActiveExpiredUsersFilter()
+            ->addUserIdsFilter($userIds);
+        $this->processExpiredUsers($expiredRecords);
+    }
 
+    /**
+     * Deactivate expired user accounts and invalidate their sessions.
+     *
+     * @param ExpiredUsersCollection $expiredRecords
+     */
+    private function processExpiredUsers(ExpiredUsersCollection $expiredRecords): void
+    {
         if ($expiredRecords->getSize() > 0) {
             // get all active sessions for the users and set them to logged out
             /** @var \Magento\Security\Model\ResourceModel\AdminSessionInfo\Collection $currentSessions */

diff --git a/app/code/Magento/Security/Observer/AdminUserAuthenticateBefore.php b/app/code/Magento/Security/Observer/AdminUserAuthenticateBefore.php
@@ -58,7 +58,7 @@ public function execute(Observer $observer)
         $user->loadByUsername($username);
 
         if ($user->getId() && $this->userExpirationManager->isUserExpired($user->getId())) {
-            $this->userExpirationManager->deactivateExpiredUsers([$user->getId()]);
+            $this->userExpirationManager->deactivateExpiredUsersById([$user->getId()]);
             throw new AuthenticationException(
                 __(
                     'The account sign-in was incorrect or your account is disabled temporarily. '

diff --git a/app/code/Magento/Security/Test/Unit/Model/Plugin/AuthSessionTest.php b/app/code/Magento/Security/Test/Unit/Model/Plugin/AuthSessionTest.php
@@ -79,7 +79,7 @@ public function setUp()
 
         $this->userExpirationManagerMock = $this->createPartialMock(
             \Magento\Security\Model\UserExpirationManager::class,
-            ['isUserExpired', 'deactivateExpiredUsers']
+            ['isUserExpired', 'deactivateExpiredUsersById']
         );
 
         $this->userMock = $this->createMock(\Magento\User\Model\User::class);
@@ -207,7 +207,7 @@ public function testAroundProlongSessionIsActiveUserIsExpired()
             ->willReturn(true);
 
         $this->userExpirationManagerMock->expects($this->once())
-            ->method('deactivateExpiredUsers')
+            ->method('deactivateExpiredUsersById')
             ->with([$adminUserId]);
 
         $this->authSessionMock->expects($this->once())

diff --git a/app/code/Magento/Security/Test/Unit/Observer/AdminUserAuthenticateBeforeTest.php b/app/code/Magento/Security/Test/Unit/Observer/AdminUserAuthenticateBeforeTest.php
@@ -65,7 +65,7 @@ protected function setUp()
 
         $this->userExpirationManagerMock = $this->createPartialMock(
             \Magento\Security\Model\UserExpirationManager::class,
-            ['isUserExpired', 'deactivateExpiredUsers']
+            ['isUserExpired', 'deactivateExpiredUsersById']
         );
         $this->userFactoryMock = $this->createPartialMock(\Magento\User\Model\UserFactory::class, ['create']);
         $this->userMock = $this->createPartialMock(\Magento\User\Model\User::class, ['loadByUsername', 'getId']);
@@ -104,7 +104,7 @@ public function testWithExpiredUser()
             ->willReturn(true);
         $this->userMock->expects(static::exactly(3))->method('getId')->willReturn($adminUserId);
         $this->userExpirationManagerMock->expects(static::once())
-            ->method('deactivateExpiredUsers')
+            ->method('deactivateExpiredUsersById')
             ->with([$adminUserId])
             ->willReturn(null);
         $this->observer->execute($this->eventObserverMock);

diff --git a/dev/tests/integration/testsuite/Magento/Security/Model/UserExpirationManagerTest.php b/dev/tests/integration/testsuite/Magento/Security/Model/UserExpirationManagerTest.php
@@ -71,7 +71,7 @@ public function testDeactivateExpiredUsersWithExpiredUser()
         $user = $this->loadUserByUsername($adminUsernameFromFixture);
         $sessionId = $this->authSession->getSessionId();
         $this->expireUser($user);
-        $this->userExpirationManager->deactivateExpiredUsers([$user->getId()]);
+        $this->userExpirationManager->deactivateExpiredUsersById([$user->getId()]);
         $this->adminSessionInfo->load($sessionId, 'session_id');
         $user->reload();
         $userExpirationModel = $this->loadExpiredUserModelByUser($user);
@@ -90,7 +90,7 @@ public function testDeactivateExpiredUsersWithNonExpiredUser()
         $this->loginUser($adminUsernameFromFixture);
         $user = $this->loadUserByUsername($adminUsernameFromFixture);
         $sessionId = $this->authSession->getSessionId();
-        $this->userExpirationManager->deactivateExpiredUsers([$user->getId()]);
+        $this->userExpirationManager->deactivateExpiredUsersById([$user->getId()]);
         $user->reload();
         $userExpirationModel = $this->loadExpiredUserModelByUser($user);
         $this->adminSessionInfo->load($sessionId, 'session_id');