diff --git a/app/code/Magento/Security/Model/AdminSessionInfo.php b/app/code/Magento/Security/Model/AdminSessionInfo.php index c14d399e98cac..b0347fdbc3add 100644 --- a/app/code/Magento/Security/Model/AdminSessionInfo.php +++ b/app/code/Magento/Security/Model/AdminSessionInfo.php @@ -47,6 +47,7 @@ class AdminSessionInfo extends \Magento\Framework\Model\AbstractModel /** * All other open sessions were terminated * @since 100.1.0 + * @var bool */ protected $isOtherSessionsTerminated = false; @@ -133,10 +134,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 154760eb835d1..e6b7a076e1c4a 100644 --- a/app/code/Magento/Security/Model/AdminSessionsManager.php +++ b/app/code/Magento/Security/Model/AdminSessionsManager.php @@ -7,7 +7,10 @@ namespace Magento\Security\Model; +use Magento\Backend\Model\Auth\Session; use Magento\Framework\HTTP\PhpEnvironment\RemoteAddress; +use Magento\Framework\Stdlib\DateTime; +use Magento\Security\Model\ResourceModel\AdminSessionInfo\Collection; use Magento\Security\Model\ResourceModel\AdminSessionInfo\CollectionFactory; /** @@ -15,6 +18,7 @@ * * @api * @since 100.1.0 + * @SuppressWarnings(PHPMD.CookieAndSessionMisuse) */ class AdminSessionsManager { @@ -35,7 +39,7 @@ class AdminSessionsManager protected $securityConfig; /** - * @var \Magento\Backend\Model\Auth\Session + * @var Session * @since 100.1.0 */ protected $authSession; @@ -73,12 +77,14 @@ class AdminSessionsManager * * Means that after session was prolonged * all other prolongs will be ignored within this period + * + * @var int */ private $maxIntervalBetweenConsecutiveProlongs = 60; /** * @param ConfigInterface $securityConfig - * @param \Magento\Backend\Model\Auth\Session $authSession + * @param Session $authSession * @param AdminSessionInfoFactory $adminSessionInfoFactory * @param CollectionFactory $adminSessionInfoCollectionFactory * @param \Magento\Framework\Stdlib\DateTime\DateTime $dateTime @@ -86,7 +92,7 @@ class AdminSessionsManager */ public function __construct( ConfigInterface $securityConfig, - \Magento\Backend\Model\Auth\Session $authSession, + Session $authSession, \Magento\Security\Model\AdminSessionInfoFactory $adminSessionInfoFactory, \Magento\Security\Model\ResourceModel\AdminSessionInfo\CollectionFactory $adminSessionInfoCollectionFactory, \Magento\Framework\Stdlib\DateTime\DateTime $dateTime, @@ -138,7 +144,7 @@ public function processProlong() $this->getCurrentSession()->setData( 'updated_at', date( - \Magento\Framework\Stdlib\DateTime::DATETIME_PHP_FORMAT, + DateTime::DATETIME_PHP_FORMAT, $this->authSession->getUpdatedAt() ) ); @@ -204,7 +210,7 @@ public function getLogoutReasonMessageByStatus($statusCode) case AdminSessionInfo::LOGGED_OUT_BY_LOGIN: $reasonMessage = __( 'Someone logged into this account from another device or browser.' - .' Your current session is terminated.' + . ' Your current session is terminated.' ); break; case AdminSessionInfo::LOGGED_OUT_MANUALLY: @@ -241,7 +247,7 @@ public function getLogoutReasonMessage() /** * Get sessions for current user * - * @return \Magento\Security\Model\ResourceModel\AdminSessionInfo\Collection + * @return Collection * @since 100.1.0 */ public function getSessionsForCurrentUser() @@ -314,7 +320,9 @@ protected function createNewSession() } /** - * @return \Magento\Security\Model\ResourceModel\AdminSessionInfo\Collection + * Retrieve new instance of admin session info collection + * + * @return Collection * @since 100.1.0 */ protected function createAdminSessionInfoCollection() @@ -323,24 +331,27 @@ 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 * X - is calculated in getIntervalBetweenConsecutiveProlongs() * - * @see getIntervalBetweenConsecutiveProlongs() * @return bool + * @see getIntervalBetweenConsecutiveProlongs() */ 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; - return (float) $diff > $this->getIntervalBetweenConsecutiveProlongs(); + return (float)$diff > $this->getIntervalBetweenConsecutiveProlongs(); } /** @@ -354,7 +365,7 @@ private function lastProlongIsOldEnough() */ private function getIntervalBetweenConsecutiveProlongs() { - return (float) max( + return (float)max( 1, min( 4 * log((float)$this->securityConfig->getAdminSessionLifetime()), diff --git a/app/code/Magento/Security/Test/Unit/Model/AdminSessionInfoTest.php b/app/code/Magento/Security/Test/Unit/Model/AdminSessionInfoTest.php index db65204811a5e..95e84afebc783 100644 --- a/app/code/Magento/Security/Test/Unit/Model/AdminSessionInfoTest.php +++ b/app/code/Magento/Security/Test/Unit/Model/AdminSessionInfoTest.php @@ -127,6 +127,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 e352583ea575f..21a8f6eaef26c 100644 --- a/app/code/Magento/Security/Test/Unit/Model/AdminSessionsManagerTest.php +++ b/app/code/Magento/Security/Test/Unit/Model/AdminSessionsManagerTest.php @@ -58,9 +58,7 @@ class AdminSessionsManagerTest extends TestCase /** @var ObjectManager */ protected $objectManager; - /* - * @var RemoteAddress - */ + /** @var RemoteAddress */ protected $remoteAddressMock; /** @@ -72,7 +70,15 @@ protected function setUp(): void $this->objectManager = new ObjectManager($this); $this->authSessionMock = $this->getMockBuilder(Session::class) - ->addMethods(['isActive', 'getStatus', 'getUser', 'getId', 'getUpdatedAt', 'getAdminSessionInfoId', 'setAdminSessionInfoId']) + ->addMethods([ + 'isActive', + 'getStatus', + 'getUser', + 'getId', + 'getUpdatedAt', + 'getAdminSessionInfoId', + 'setAdminSessionInfoId' + ]) ->disableOriginalConstructor() ->getMock(); @@ -255,6 +261,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 */