Skip to content

Commit 6407af8

Browse files
lfolcolfolco
committed
handle dates with store time (#22833: Short-term admin accounts)
1 parent b631f9d commit 6407af8

File tree

7 files changed

+76
-56
lines changed

7 files changed

+76
-56
lines changed

‎app/code/Magento/Security/Model/ResourceModel/UserExpiration.php

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,35 @@ class UserExpiration extends \Magento\Framework\Model\ResourceModel\Db\AbstractD
2020
*/
2121
protected $_isPkAutoIncrement = false;
2222

23+
/**
24+
* @var \Magento\Framework\Stdlib\DateTime\TimezoneInterface
25+
*/
26+
private $timezone;
27+
28+
/**
29+
* @var \Magento\Framework\Stdlib\DateTime\DateTime
30+
*/
31+
private $dateTime;
32+
33+
/**
34+
* UserExpiration constructor.
35+
*
36+
* @param \Magento\Framework\Model\ResourceModel\Db\Context $context
37+
* @param \Magento\Framework\Stdlib\DateTime\TimezoneInterface $timezone
38+
* @param \Magento\Framework\Stdlib\DateTime\DateTime $dateTime
39+
* @param null $connectionName
40+
*/
41+
public function __construct(
42+
\Magento\Framework\Model\ResourceModel\Db\Context $context,
43+
\Magento\Framework\Stdlib\DateTime\TimezoneInterface $timezone,
44+
\Magento\Framework\Stdlib\DateTime\DateTime $dateTime,
45+
$connectionName = null
46+
) {
47+
parent::__construct($context, $connectionName);
48+
$this->timezone = $timezone;
49+
$this->dateTime = $dateTime;
50+
}
51+
2352
/**
2453
* Define main table
2554
*
@@ -31,19 +60,34 @@ protected function _construct()
3160
}
3261

3362
/**
34-
* Perform actions before object save
63+
* Convert to UTC time.
3564
*
36-
* @param \Magento\Framework\Model\AbstractModel $object
65+
* @param \Magento\Framework\Model\AbstractModel $userExpiration
3766
* @return $this
38-
* @throws \Magento\Framework\Exception\LocalizedException
3967
*/
40-
public function _beforeSave(\Magento\Framework\Model\AbstractModel $object)
68+
protected function _beforeSave(\Magento\Framework\Model\AbstractModel $userExpiration)
4169
{
42-
/** @var $object \Magento\Security\Model\UserExpiration */
43-
if ($object->getExpiresAt() instanceof \DateTimeInterface) {
70+
/** @var $userExpiration \Magento\Security\Model\UserExpiration */
71+
$expiresAt = $userExpiration->getExpiresAt();
72+
$utcValue = $this->dateTime->gmtDate('Y-m-d H:i:s', $expiresAt);
73+
$userExpiration->setExpiresAt($utcValue);
74+
75+
return $this;
76+
}
4477

45-
// TODO: use this? need to check if we're ever passing in a \DateTimeInterface or if it's always a string
46-
$object->setExpiresAt($object->getExpiresAt()->format('Y-m-d H:i:s'));
78+
/**
79+
* Convert to store time.
80+
*
81+
* @param \Magento\Framework\Model\AbstractModel $userExpiration
82+
* @return $this|\Magento\Framework\Model\ResourceModel\Db\AbstractDb
83+
* @throws \Exception
84+
*/
85+
protected function _afterLoad(\Magento\Framework\Model\AbstractModel $userExpiration)
86+
{
87+
/** @var $userExpiration \Magento\Security\Model\UserExpiration */
88+
if ($userExpiration->getExpiresAt()) {
89+
$storeValue = $this->timezone->date($userExpiration->getExpiresAt());
90+
$userExpiration->setExpiresAt($storeValue);
4791
}
4892

4993
return $this;

‎app/code/Magento/Security/Model/UserExpiration.php

Lines changed: 1 addition & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -10,38 +10,11 @@
1010
/**
1111
* Admin User Expiration model.
1212
* @method string getExpiresAt()
13-
* @method \Magento\Security\Model\UserExpiration setExpiresAt(string $value)
13+
* @method \Magento\Security\Model\UserExpiration setExpiresAt($value)
1414
*/
1515
class UserExpiration extends \Magento\Framework\Model\AbstractModel
1616
{
1717

18-
/**
19-
* @var UserExpiration\Validator
20-
*/
21-
private $validator;
22-
23-
/**
24-
* UserExpiration constructor.
25-
*
26-
* @param \Magento\Framework\Model\Context $context
27-
* @param \Magento\Framework\Registry $registry
28-
* @param UserExpiration\Validator $validator
29-
* @param \Magento\Framework\Model\ResourceModel\AbstractResource|null $resource
30-
* @param \Magento\Framework\Data\Collection\AbstractDb|null $resourceCollection
31-
* @param array $data
32-
*/
33-
public function __construct(
34-
\Magento\Framework\Model\Context $context,
35-
\Magento\Framework\Registry $registry,
36-
\Magento\Security\Model\UserExpiration\Validator $validator,
37-
\Magento\Framework\Model\ResourceModel\AbstractResource $resource = null,
38-
\Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null,
39-
array $data = []
40-
) {
41-
parent::__construct($context, $registry, $resource, $resourceCollection, $data);
42-
$this->validator = $validator;
43-
}
44-
4518
/**
4619
* Resource initialization
4720
*
@@ -51,12 +24,4 @@ protected function _construct()
5124
{
5225
$this->_init(\Magento\Security\Model\ResourceModel\UserExpiration::class);
5326
}
54-
55-
/**
56-
* TODO: remove and use a plugin on UserValidationRules
57-
*/
58-
// protected function _getValidationRulesBeforeSave()
59-
// {
60-
// return $this->validator;
61-
// }
6227
}

‎app/code/Magento/Security/Model/UserExpiration/Validator.php

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,15 @@
1616
*/
1717
class Validator extends AbstractValidator
1818
{
19+
/**@var \Magento\Framework\Stdlib\DateTime\DateTime */
20+
private $dateTime;
21+
22+
public function __construct(
23+
\Magento\Framework\Stdlib\DateTime\DateTime $dateTime
24+
) {
25+
26+
$this->dateTime = $dateTime;
27+
}
1928

2029
/**
2130
* Ensure that the given date is later than the current date.
@@ -31,9 +40,8 @@ public function isValid($value)
3140
$expiresAt = $value;
3241
$label = 'Expiration date';
3342
if (\Zend_Validate::is($expiresAt, 'NotEmpty')) {
34-
$currentTime = new \DateTime();
35-
$expiresAt = new \DateTime($expiresAt);
36-
43+
$currentTime = $this->dateTime->gmtTimestamp();
44+
$expiresAt = $this->dateTime->gmtTimestamp($value);
3745
if ($expiresAt < $currentTime) {
3846
$messages['expires_at'] = __('"%1" must be later than the current date.', $label);
3947
}

‎app/code/Magento/Security/Model/UserExpirationManager.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
use Magento\Security\Model\ResourceModel\UserExpiration\Collection as ExpiredUsersCollection;
1111

1212
/**
13-
* Class to handle expired admin users.
13+
* Class to handle admin user expirations.
1414
*/
1515
class UserExpirationManager
1616
{
@@ -107,7 +107,6 @@ public function deactivateExpiredUsers(?array $userIds = null): void
107107

108108
/**
109109
* Check if the given user is expired.
110-
* // TODO: check users expired an hour ago (timezone stuff)
111110
*
112111
* @param \Magento\User\Model\User $user
113112
* @return bool

‎app/code/Magento/Security/Observer/AdminUserAuthenticateBefore.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public function execute(Observer $observer)
5656
/** @var \Magento\User\Model\User $user */
5757
$user = $this->user->loadByUsername($username);
5858

59-
if ($this->userExpirationManager->userIsExpired($user)) {
59+
if ($user->getId() && $this->userExpirationManager->userIsExpired($user)) {
6060
$this->userExpirationManager->deactivateExpiredUsers([$user->getId()]);
6161
throw new AuthenticationException(
6262
__(

‎app/code/Magento/User/Block/User/Edit/Tab/Main.php

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,16 +169,22 @@ protected function _prepareForm()
169169
]
170170
);
171171
}
172-
// TODO: use store time and convert to GMT
172+
173+
$dateFormat = $this->_localeDate->getDateFormat(
174+
\IntlDateFormatter::MEDIUM
175+
);
176+
$timeFormat = $this->_localeDate->getTimeFormat(
177+
\IntlDateFormatter::MEDIUM
178+
);
173179
$baseFieldset->addField(
174180
'expires_at',
175181
'date',
176182
[
177183
'name' => 'expires_at',
178184
'label' => __('Expiration Date'),
179185
'title' => __('Expiration Date'),
180-
'date_format' => 'yyyy-MM-dd',
181-
'time_format' => 'hh:mm:ss',
186+
'date_format' => $dateFormat,
187+
'time_format' => $timeFormat,
182188
'class' => 'validate-date',
183189
]
184190
);

‎dev/tests/integration/testsuite/Magento/Security/Model/UserExpirationManagerTest.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ protected function setUp()
4444
->setCurrentScope(\Magento\Backend\App\Area\FrontNameResolver::AREA_CODE);
4545
$this->auth = $this->objectManager->create(\Magento\Backend\Model\Auth::class);
4646
$this->authSession = $this->objectManager->create(\Magento\Backend\Model\Auth\Session::class);
47-
$this->auth->setAuthStorage($this->authSession);
4847
$this->adminSessionInfo = $this->objectManager->create(\Magento\Security\Model\AdminSessionInfo::class);
48+
$this->auth->setAuthStorage($this->authSession);
4949
$this->userExpirationManager =
5050
$this->objectManager->create(\Magento\Security\Model\UserExpirationManager::class);
5151
}
@@ -67,7 +67,6 @@ protected function tearDown()
6767
*/
6868
public function testUserIsExpired()
6969
{
70-
static::markTestSkipped();
7170
$adminUserNameFromFixture = 'adminUserExpired';
7271
$user = $this->loadUserByUsername($adminUserNameFromFixture);
7372
static::assertTrue($this->userExpirationManager->userIsExpired($user));
@@ -121,7 +120,6 @@ public function testDeactivateExpiredUsersWithNonExpiredUser()
121120
*/
122121
public function testDeactivateExpiredUsers()
123122
{
124-
static::markTestSkipped();
125123
$notExpiredUser = $this->loadUserByUsername('adminUserNotExpired');
126124
$expiredUser = $this->loadUserByUsername('adminUserExpired');
127125
$this->userExpirationManager->deactivateExpiredUsers();

0 commit comments

Comments
 (0)