Skip to content

Commit fd191fe

Browse files
ENGCOM-9336: AdminSessionsManager and AdminSessionInfo - strtotime issue fix for admin login PR #34514
- Merge Pull Request #34514 from kanhaiya5590/magento2:2.4-develop - Merged commits: 1. 51b2821 2. 7af375e 3. 5e885af 4. 47f6999 5. 86de3f6 6. a815765 7. d951119 8. 15b1b8b 9. e7b400b 10. d50d992 11. 29cd307 12. fd6382b 13. fa82a64 14. 06249d5 15. 854df7e 16. b8cfbe1 17. bd4996f 18. 1918bc2 19. 14b5d3c
2 parents 1d1614b + 14b5d3c commit fd191fe

File tree

4 files changed

+99
-19
lines changed

4 files changed

+99
-19
lines changed

app/code/Magento/Security/Model/AdminSessionInfo.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ class AdminSessionInfo extends \Magento\Framework\Model\AbstractModel
4747
/**
4848
* All other open sessions were terminated
4949
* @since 100.1.0
50+
* @var bool
5051
*/
5152
protected $isOtherSessionsTerminated = false;
5253

@@ -133,10 +134,10 @@ public function isSessionExpired()
133134
$currentTime = $this->dateTime->gmtTimestamp();
134135
$lastUpdatedTime = $this->getUpdatedAt();
135136
if (!is_numeric($lastUpdatedTime)) {
136-
$lastUpdatedTime = strtotime($lastUpdatedTime);
137+
$lastUpdatedTime = $lastUpdatedTime === null ? 0 : strtotime($lastUpdatedTime);
137138
}
138139

139-
return $lastUpdatedTime <= ($currentTime - $lifetime) ? true : false;
140+
return $lastUpdatedTime <= ($currentTime - $lifetime);
140141
}
141142

142143
/**

app/code/Magento/Security/Model/AdminSessionsManager.php

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,18 @@
77

88
namespace Magento\Security\Model;
99

10+
use Magento\Backend\Model\Auth\Session;
1011
use Magento\Framework\HTTP\PhpEnvironment\RemoteAddress;
12+
use Magento\Framework\Stdlib\DateTime;
13+
use Magento\Security\Model\ResourceModel\AdminSessionInfo\Collection;
1114
use Magento\Security\Model\ResourceModel\AdminSessionInfo\CollectionFactory;
1215

1316
/**
1417
* Admin Sessions Manager Model
1518
*
1619
* @api
1720
* @since 100.1.0
21+
* @SuppressWarnings(PHPMD.CookieAndSessionMisuse)
1822
*/
1923
class AdminSessionsManager
2024
{
@@ -35,7 +39,7 @@ class AdminSessionsManager
3539
protected $securityConfig;
3640

3741
/**
38-
* @var \Magento\Backend\Model\Auth\Session
42+
* @var Session
3943
* @since 100.1.0
4044
*/
4145
protected $authSession;
@@ -73,20 +77,22 @@ class AdminSessionsManager
7377
*
7478
* Means that after session was prolonged
7579
* all other prolongs will be ignored within this period
80+
*
81+
* @var int
7682
*/
7783
private $maxIntervalBetweenConsecutiveProlongs = 60;
7884

7985
/**
8086
* @param ConfigInterface $securityConfig
81-
* @param \Magento\Backend\Model\Auth\Session $authSession
87+
* @param Session $authSession
8288
* @param AdminSessionInfoFactory $adminSessionInfoFactory
8389
* @param CollectionFactory $adminSessionInfoCollectionFactory
8490
* @param \Magento\Framework\Stdlib\DateTime\DateTime $dateTime
8591
* @param RemoteAddress $remoteAddress
8692
*/
8793
public function __construct(
8894
ConfigInterface $securityConfig,
89-
\Magento\Backend\Model\Auth\Session $authSession,
95+
Session $authSession,
9096
\Magento\Security\Model\AdminSessionInfoFactory $adminSessionInfoFactory,
9197
\Magento\Security\Model\ResourceModel\AdminSessionInfo\CollectionFactory $adminSessionInfoCollectionFactory,
9298
\Magento\Framework\Stdlib\DateTime\DateTime $dateTime,
@@ -138,7 +144,7 @@ public function processProlong()
138144
$this->getCurrentSession()->setData(
139145
'updated_at',
140146
date(
141-
\Magento\Framework\Stdlib\DateTime::DATETIME_PHP_FORMAT,
147+
DateTime::DATETIME_PHP_FORMAT,
142148
$this->authSession->getUpdatedAt()
143149
)
144150
);
@@ -204,7 +210,7 @@ public function getLogoutReasonMessageByStatus($statusCode)
204210
case AdminSessionInfo::LOGGED_OUT_BY_LOGIN:
205211
$reasonMessage = __(
206212
'Someone logged into this account from another device or browser.'
207-
.' Your current session is terminated.'
213+
. ' Your current session is terminated.'
208214
);
209215
break;
210216
case AdminSessionInfo::LOGGED_OUT_MANUALLY:
@@ -241,7 +247,7 @@ public function getLogoutReasonMessage()
241247
/**
242248
* Get sessions for current user
243249
*
244-
* @return \Magento\Security\Model\ResourceModel\AdminSessionInfo\Collection
250+
* @return Collection
245251
* @since 100.1.0
246252
*/
247253
public function getSessionsForCurrentUser()
@@ -314,7 +320,9 @@ protected function createNewSession()
314320
}
315321

316322
/**
317-
* @return \Magento\Security\Model\ResourceModel\AdminSessionInfo\Collection
323+
* Retrieve new instance of admin session info collection
324+
*
325+
* @return Collection
318326
* @since 100.1.0
319327
*/
320328
protected function createAdminSessionInfoCollection()
@@ -323,24 +331,27 @@ protected function createAdminSessionInfoCollection()
323331
}
324332

325333
/**
326-
* Calculates diff between now and last session updated_at
327-
* and decides whether new prolong must be triggered or not
334+
* Calculates diff between now and last session updated_at and decides whether new prolong must be triggered or not
328335
*
329336
* This is done to limit amount of session prolongs and updates to database
330337
* within some period of time - X
331338
* X - is calculated in getIntervalBetweenConsecutiveProlongs()
332339
*
333-
* @see getIntervalBetweenConsecutiveProlongs()
334340
* @return bool
341+
* @see getIntervalBetweenConsecutiveProlongs()
335342
*/
336343
private function lastProlongIsOldEnough()
337344
{
338-
$lastProlongTimestamp = strtotime($this->getCurrentSession()->getUpdatedAt());
345+
$lastUpdatedTime = $this->getCurrentSession()->getUpdatedAt();
346+
if ($lastUpdatedTime === null || is_numeric($lastUpdatedTime)) {
347+
$lastUpdatedTime = "now";
348+
}
349+
$lastProlongTimestamp = strtotime($lastUpdatedTime);
339350
$nowTimestamp = $this->authSession->getUpdatedAt();
340351

341352
$diff = $nowTimestamp - $lastProlongTimestamp;
342353

343-
return (float) $diff > $this->getIntervalBetweenConsecutiveProlongs();
354+
return (float)$diff > $this->getIntervalBetweenConsecutiveProlongs();
344355
}
345356

346357
/**
@@ -354,7 +365,7 @@ private function lastProlongIsOldEnough()
354365
*/
355366
private function getIntervalBetweenConsecutiveProlongs()
356367
{
357-
return (float) max(
368+
return (float)max(
358369
1,
359370
min(
360371
4 * log((float)$this->securityConfig->getAdminSessionLifetime()),

app/code/Magento/Security/Test/Unit/Model/AdminSessionInfoTest.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,26 @@ public function dataProviderSessionLifetime()
127127
];
128128
}
129129

130+
/**
131+
* @return void
132+
*/
133+
public function testSessionExpiredWhenUpdatedAtIsNull()
134+
{
135+
$timestamp = time();
136+
$sessionLifetime = '1';
137+
138+
$this->securityConfigMock->expects($this->once())
139+
->method('getAdminSessionLifetime')
140+
->willReturn($sessionLifetime);
141+
142+
$this->dateTimeMock->expects($this->once())
143+
->method('gmtTimestamp')
144+
->willReturn($timestamp);
145+
146+
$this->model->setUpdatedAt(null);
147+
$this->assertTrue($this->model->isSessionExpired());
148+
}
149+
130150
/**
131151
* @return void
132152
*/

app/code/Magento/Security/Test/Unit/Model/AdminSessionsManagerTest.php

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,7 @@ class AdminSessionsManagerTest extends TestCase
5858
/** @var ObjectManager */
5959
protected $objectManager;
6060

61-
/*
62-
* @var RemoteAddress
63-
*/
61+
/** @var RemoteAddress */
6462
protected $remoteAddressMock;
6563

6664
/**
@@ -72,7 +70,15 @@ protected function setUp(): void
7270
$this->objectManager = new ObjectManager($this);
7371

7472
$this->authSessionMock = $this->getMockBuilder(Session::class)
75-
->addMethods(['isActive', 'getStatus', 'getUser', 'getId', 'getUpdatedAt', 'getAdminSessionInfoId', 'setAdminSessionInfoId'])
73+
->addMethods([
74+
'isActive',
75+
'getStatus',
76+
'getUser',
77+
'getId',
78+
'getUpdatedAt',
79+
'getAdminSessionInfoId',
80+
'setAdminSessionInfoId'
81+
])
7682
->disableOriginalConstructor()
7783
->getMock();
7884

@@ -255,6 +261,48 @@ public function testProcessProlong()
255261
$this->model->processProlong();
256262
}
257263

264+
/**
265+
* @return void
266+
*/
267+
public function testUpdatedAtIsNull()
268+
{
269+
$newUpdatedAt = '2016-01-01 00:00:30';
270+
$adminSessionInfoId = 50;
271+
$this->authSessionMock->expects($this->any())
272+
->method('getAdminSessionInfoId')
273+
->willReturn($adminSessionInfoId);
274+
275+
$this->adminSessionInfoFactoryMock->expects($this->any())
276+
->method('create')
277+
->willReturn($this->currentSessionMock);
278+
279+
$this->currentSessionMock->expects($this->once())
280+
->method('load')
281+
->willReturnSelf();
282+
283+
$this->currentSessionMock->expects($this->once())
284+
->method('getUpdatedAt')
285+
->willReturn(null);
286+
287+
$this->authSessionMock->expects($this->once())
288+
->method('getUpdatedAt')
289+
->willReturn(strtotime($newUpdatedAt));
290+
291+
$this->securityConfigMock->expects($this->once())
292+
->method('getAdminSessionLifetime')
293+
->willReturn(100);
294+
295+
$this->currentSessionMock->expects($this->never())
296+
->method('setData')
297+
->willReturnSelf();
298+
299+
$this->currentSessionMock->expects($this->never())
300+
->method('save')
301+
->willReturnSelf();
302+
303+
$this->model->processProlong();
304+
}
305+
258306
/**
259307
* @return void
260308
*/

0 commit comments

Comments
 (0)