deactivate expired users on session prolong (magento#22833)

lfolco · Oct 19, 2019 · babc965 · babc965
1 parent df0c97c
commit babc965
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 10 deletions.
diff --git a/app/code/Magento/Security/Model/Plugin/AuthSession.php b/app/code/Magento/Security/Model/Plugin/AuthSession.php
@@ -71,8 +71,12 @@ public function __construct(
      */
     public function aroundProlong(Session $session, \Closure $proceed)
     {
-        if (!$this->sessionsManager->getCurrentSession()->isLoggedInStatus() ||
-            $this->userExpirationManager->isUserExpired($session->getUser()->getId())) {
+        if (!$this->sessionsManager->getCurrentSession()->isLoggedInStatus()) {
+            $session->destroy();
+            $this->addUserLogoutNotification();
+            return null;
+        } elseif ($this->userExpirationManager->isUserExpired($session->getUser()->getId())) {
+            $this->userExpirationManager->deactivateExpiredUsers([$session->getUser()->getId()]);
             $session->destroy();
             $this->addUserLogoutNotification();
             return null;

diff --git a/app/code/Magento/Security/Test/Mftf/Test/AdminNavigateWhileUserExpiredTest.xml b/app/code/Magento/Security/Test/Mftf/Test/AdminNavigateWhileUserExpiredTest.xml
@@ -0,0 +1,53 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<tests xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xsi:noNamespaceSchemaLocation="urn:magento:mftf:Test/etc/testSchema.xsd">
+
+    <test name="AdminNavigateWhileUserExpiredTest">
+        <annotations>
+            <features value="Security"/>
+            <stories value="Navigate to an admin page after user expiration date passes."/>
+            <title value="Navigate to an admin page after user expiration date passes"/>
+            <description value="Navigate to an admin page after user expiration date passes."/>
+            <testCaseId value="" />
+            <severity value="CRITICAL"/>
+            <group value="security"/>
+        </annotations>
+
+        <before>
+            <actionGroup ref="LoginAsAdmin" stepKey="loginAsAdmin"/>
+        </before>
+        <after>
+            <actionGroup ref="logout" stepKey="logout"/>
+        </after>
+
+        <!-- Create user -->
+        <actionGroup ref="AdminOpenNewUserPageActionGroup" stepKey="openNewUserPage" />
+        <generateDate date="+2 minute" format="M d, Y g:i:s A" stepKey="expiresDateTime"/>
+        <actionGroup ref="AdminFillInUserWithExpirationActionGroup" stepKey="fillInNewUserWithValidExpiration">
+            <argument name="expires_at" value="{$expiresDateTime}"/>
+        </actionGroup>
+        <grabValueFrom selector="{{AdminNewUserFormSection.username}}" stepKey="grabUsername"/>
+        <grabValueFrom selector="{{AdminNewUserFormSection.password}}" stepKey="grabPassword"/>
+        <scrollToTopOfPage stepKey="scrollToTopOfPage"/>
+        <click selector="{{AdminNewUserFormSection.userInfoTab}}" stepKey="openUserInfoTab"/>
+        <actionGroup ref="AdminSaveUserSuccessActionGroup" stepKey="saveNewUserWithValidExpirationSuccess"/>
+        <actionGroup ref="logout" stepKey="logout"/>
+
+        <!-- Login as that user -->
+        <actionGroup ref="LoginAdminWithCredentialsActionGroup" stepKey="loginAsNewAdmin">
+            <argument name="adminUser" value="{$grabUsername}"/>
+            <argument name="adminPassword" value="{$grabPassword}"/>
+        </actionGroup>
+        <actionGroup ref="AssertAdminDashboardPageIsVisibleActionGroup" stepKey="seeDashboardPage"/>
+        <wait time="120" stepKey="waitForUserToExpire"/>
+        <amOnPage url="{{AdminCustomerPage.url}}" stepKey="navigateToCustomers"/>
+        <!-- Confirm that user is logged out -->
+        <seeInCurrentUrl url="{{AdminLoginPage.url}}" stepKey="seeAdminLoginUrl"/>
+
+        <!-- Delete created user -->
+        <actionGroup ref="LoginAsAdmin" stepKey="loginAsAdmin"/>
+        <actionGroup ref="AdminDeleteCustomUserActionGroup" stepKey="deleteUser">
+            <argument name="user" value="NewAdminUser"/>
+        </actionGroup>
+    </test>
+</tests>
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']
+            ['isUserExpired', 'deactivateExpiredUsers']
         );
 
         $this->userMock = $this->createMock(\Magento\User\Model\User::class);
@@ -188,27 +188,31 @@ public function testAroundProlongSessionIsActiveUserIsExpired()
             ->method('isLoggedInStatus')
             ->willReturn(true);
 
-        $this->authSessionMock->expects($this->once())
+        $this->authSessionMock->expects($this->exactly(2))
             ->method('getUser')
             ->willReturn($this->userMock);
 
-        $this->userMock->expects($this->once())
+        $this->userMock->expects($this->exactly(2))
             ->method('getId')
             ->willReturn($adminUserId);
 
+        $this->requestMock->expects($this->once())
+            ->method('getParam')
+            ->with('isAjax')
+            ->willReturn(false);
+
         $this->userExpirationManagerMock->expects($this->once())
             ->method('isUserExpired')
             ->with($adminUserId)
             ->willReturn(true);
 
+        $this->userExpirationManagerMock->expects($this->once())
+            ->method('deactivateExpiredUsers')
+            ->with([$adminUserId]);
+
         $this->authSessionMock->expects($this->once())
             ->method('destroy');
 
-        $this->requestMock->expects($this->once())
-            ->method('getParam')
-            ->with('isAjax')
-            ->willReturn(false);
-
         $this->adminSessionsManagerMock->expects($this->once())
             ->method('getLogoutReasonMessage')
             ->willReturn($errorMessage);

diff --git a/dev/tests/integration/testsuite/Magento/Security/Model/Plugin/AuthSessionTest.php b/dev/tests/integration/testsuite/Magento/Security/Model/Plugin/AuthSessionTest.php
@@ -156,6 +156,7 @@ public function testProcessProlongWithExpiredUser()
 
         $expireDate = new \DateTime();
         $expireDate->modify('-10 days');
+        /** @var \Magento\User\Model\User $user */
         $user = $this->objectManager->create(\Magento\User\Model\User::class);
         $user->loadByUsername(\Magento\TestFramework\Bootstrap::ADMIN_NAME);
         $userExpirationFactory = $this->objectManager->create(\Magento\Security\Model\UserExpirationFactory::class);
@@ -178,5 +179,7 @@ public function testProcessProlongWithExpiredUser()
         $this->adminSessionInfo->load($sessionId, 'session_id');
         $this->authSession->prolong();
         static::assertFalse($this->auth->isLoggedIn());
+        $user->reload();
+        static::assertFalse((bool)$user->getIsActive());
     }
 }