diff --git a/app/code/Magento/Security/Model/AdminSessionInfo.php b/app/code/Magento/Security/Model/AdminSessionInfo.php index 77d864965baca..1ad9ff8c442cf 100644 --- a/app/code/Magento/Security/Model/AdminSessionInfo.php +++ b/app/code/Magento/Security/Model/AdminSessionInfo.php @@ -46,6 +46,7 @@ class AdminSessionInfo extends \Magento\Framework\Model\AbstractModel /** * All other open sessions were terminated * @since 100.1.0 + * @var bool */ protected $isOtherSessionsTerminated = false; @@ -132,10 +133,10 @@ public function isSessionExpired() $currentTime = $this->dateTime->gmtTimestamp(); $lastUpdatedTime = $this->getUpdatedAt(); if (!is_numeric($lastUpdatedTime)) { - $lastUpdatedTime = strtotime($lastUpdatedTime); + $lastUpdatedTime = $lastUpdatedTime === null ? 0 : strtotime($lastUpdatedTime); } - return $lastUpdatedTime <= ($currentTime - $lifetime) ? true : false; + return $lastUpdatedTime <= ($currentTime - $lifetime); } /** diff --git a/app/code/Magento/Security/Model/AdminSessionsManager.php b/app/code/Magento/Security/Model/AdminSessionsManager.php index 7503fe04ba480..ac6e1c1be8b71 100644 --- a/app/code/Magento/Security/Model/AdminSessionsManager.php +++ b/app/code/Magento/Security/Model/AdminSessionsManager.php @@ -14,6 +14,7 @@ * * @api * @since 100.1.0 + * @SuppressWarnings(PHPMD.CookieAndSessionMisuse) */ class AdminSessionsManager { @@ -305,6 +306,8 @@ protected function createNewSession() } /** + * Retrieve new instance of admin session info collection + * * @return \Magento\Security\Model\ResourceModel\AdminSessionInfo\Collection * @since 100.1.0 */ @@ -314,8 +317,7 @@ protected function createAdminSessionInfoCollection() } /** - * Calculates diff between now and last session updated_at - * and decides whether new prolong must be triggered or not + * Calculates diff between now and last session updated_at and decides whether new prolong must be triggered or not * * This is done to limit amount of session prolongs and updates to database * within some period of time - X @@ -326,7 +328,11 @@ protected function createAdminSessionInfoCollection() */ private function lastProlongIsOldEnough() { - $lastProlongTimestamp = strtotime($this->getCurrentSession()->getUpdatedAt()); + $lastUpdatedTime = $this->getCurrentSession()->getUpdatedAt(); + if ($lastUpdatedTime === null || is_numeric($lastUpdatedTime)) { + $lastUpdatedTime = "now"; + } + $lastProlongTimestamp = strtotime($lastUpdatedTime); $nowTimestamp = $this->authSession->getUpdatedAt(); $diff = $nowTimestamp - $lastProlongTimestamp; diff --git a/app/code/Magento/Security/Test/Unit/Model/AdminSessionInfoTest.php b/app/code/Magento/Security/Test/Unit/Model/AdminSessionInfoTest.php index 21a59a03bf6fe..81ce977e7720d 100644 --- a/app/code/Magento/Security/Test/Unit/Model/AdminSessionInfoTest.php +++ b/app/code/Magento/Security/Test/Unit/Model/AdminSessionInfoTest.php @@ -122,6 +122,26 @@ public function dataProviderSessionLifetime() ]; } + /** + * @return void + */ + public function testSessionExpiredWhenUpdatedAtIsNull() + { + $timestamp = time(); + $sessionLifetime = '1'; + + $this->securityConfigMock->expects($this->once()) + ->method('getAdminSessionLifetime') + ->willReturn($sessionLifetime); + + $this->dateTimeMock->expects($this->once()) + ->method('gmtTimestamp') + ->willReturn($timestamp); + + $this->model->setUpdatedAt(null); + $this->assertTrue($this->model->isSessionExpired()); + } + /** * @return void */ diff --git a/app/code/Magento/Security/Test/Unit/Model/AdminSessionsManagerTest.php b/app/code/Magento/Security/Test/Unit/Model/AdminSessionsManagerTest.php index 6f0c3e1cccf87..6a46a19c2b8cf 100644 --- a/app/code/Magento/Security/Test/Unit/Model/AdminSessionsManagerTest.php +++ b/app/code/Magento/Security/Test/Unit/Model/AdminSessionsManagerTest.php @@ -50,9 +50,7 @@ class AdminSessionsManagerTest extends \PHPUnit\Framework\TestCase /** @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager */ protected $objectManager; - /* - * @var RemoteAddress - */ + /** @var RemoteAddress */ protected $remoteAddressMock; /** @@ -256,6 +254,48 @@ public function testProcessProlong() $this->model->processProlong(); } + /** + * @return void + */ + public function testUpdatedAtIsNull() + { + $newUpdatedAt = '2016-01-01 00:00:30'; + $adminSessionInfoId = 50; + $this->authSessionMock->expects($this->any()) + ->method('getAdminSessionInfoId') + ->willReturn($adminSessionInfoId); + + $this->adminSessionInfoFactoryMock->expects($this->any()) + ->method('create') + ->willReturn($this->currentSessionMock); + + $this->currentSessionMock->expects($this->once()) + ->method('load') + ->willReturnSelf(); + + $this->currentSessionMock->expects($this->once()) + ->method('getUpdatedAt') + ->willReturn(null); + + $this->authSessionMock->expects($this->once()) + ->method('getUpdatedAt') + ->willReturn(strtotime($newUpdatedAt)); + + $this->securityConfigMock->expects($this->once()) + ->method('getAdminSessionLifetime') + ->willReturn(100); + + $this->currentSessionMock->expects($this->never()) + ->method('setData') + ->willReturnSelf(); + + $this->currentSessionMock->expects($this->never()) + ->method('save') + ->willReturnSelf(); + + $this->model->processProlong(); + } + /** * @return void */