Skip to content

Commit 5c5ad5b

Browse files
Rizwan KhanRizwan Khan
authored andcommitted
AC-9797: 2FA functionality enhancement
1 parent 5de63df commit 5c5ad5b

File tree

3 files changed

+14
-22
lines changed

3 files changed

+14
-22
lines changed

TwoFactorAuth/Controller/Adminhtml/Authy/Authpost.php

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,8 @@ class Authpost extends AbstractAction implements HttpPostActionInterface
9191
* @param TfaInterface $tfa
9292
* @param AlertInterface $alert
9393
* @param DataObjectFactory $dataObjectFactory
94-
* @param UserResource $userResource
95-
* @param ScopeConfigInterface $scopeConfig
94+
* @param UserResource|null $userResource
95+
* @param ScopeConfigInterface|null $scopeConfig
9696
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
9797
*/
9898
public function __construct(
@@ -104,8 +104,8 @@ public function __construct(
104104
TfaInterface $tfa,
105105
AlertInterface $alert,
106106
DataObjectFactory $dataObjectFactory,
107-
UserResource $userResource,
108-
ScopeConfigInterface $scopeConfig
107+
?UserResource $userResource = null,
108+
?ScopeConfigInterface $scopeConfig = null
109109
) {
110110
parent::__construct($context);
111111
$this->tfa = $tfa;
@@ -141,7 +141,7 @@ public function execute()
141141
if (!$this->allowApiRetries()) { //locked the user
142142
$lockThreshold = $this->scopeConfig->getValue(self::XML_PATH_2FA_LOCK_EXPIRE);
143143
if ($this->userResource->lock($user->getId(), 0, $lockThreshold)) {
144-
$result->setData(['success' => false, 'message' => "User is disabled temporarily!"]);
144+
$result->setData(['success' => false, 'message' => "Your account is temporarily disabled."]);
145145
return $result;
146146
}
147147
}
@@ -189,10 +189,6 @@ private function allowApiRetries() : bool
189189
$verifyAttempts = $this->session->getOtpAttempt();
190190
$verifyAttempts = $verifyAttempts === null ? 1 : $verifyAttempts+1;
191191
$this->session->setOtpAttempt($verifyAttempts);
192-
if ($verifyAttempts > $maxRetries) {
193-
return false;
194-
}
195-
196-
return true;
192+
return $maxRetries >= $verifyAttempts;
197193
}
198194
}

TwoFactorAuth/Controller/Adminhtml/Google/Authpost.php

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ class Authpost extends AbstractAction implements HttpPostActionInterface
9393
* @param TfaInterface $tfa
9494
* @param AlertInterface $alert
9595
* @param DataObjectFactory $dataObjectFactory
96-
* @param UserResource $userResource
97-
* @param ScopeConfigInterface $scopeConfig
96+
* @param UserResource|null $userResource
97+
* @param ScopeConfigInterface|null $scopeConfig
9898
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
9999
*/
100100
public function __construct(
@@ -106,8 +106,8 @@ public function __construct(
106106
TfaInterface $tfa,
107107
AlertInterface $alert,
108108
DataObjectFactory $dataObjectFactory,
109-
UserResource $userResource,
110-
ScopeConfigInterface $scopeConfig
109+
?UserResource $userResource = null,
110+
?ScopeConfigInterface $scopeConfig = null
111111
) {
112112
parent::__construct($context);
113113
$this->tfa = $tfa;
@@ -136,7 +136,7 @@ public function execute()
136136
if (!$this->allowApiRetries()) { //locked the user
137137
$lockThreshold = $this->scopeConfig->getValue(self::XML_PATH_2FA_LOCK_EXPIRE);
138138
if ($this->userResource->lock($user->getId(), 0, $lockThreshold)) {
139-
$response->setData(['success' => false, 'message' => "User is disabled temporarily!"]);
139+
$response->setData(['success' => false, 'message' => "Your account is temporarily disabled."]);
140140
return $response;
141141
}
142142
}
@@ -183,10 +183,6 @@ private function allowApiRetries() : bool
183183
$verifyAttempts = $this->session->getOtpAttempt();
184184
$verifyAttempts = $verifyAttempts === null ? 1 : $verifyAttempts+1;
185185
$this->session->setOtpAttempt($verifyAttempts);
186-
if ($verifyAttempts > $maxRetries) {
187-
return false;
188-
}
189-
190-
return true;
186+
return $maxRetries >= $verifyAttempts;
191187
}
192188
}

TwoFactorAuth/etc/adminhtml/system.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,13 @@
3737
</field>
3838
<field canRestore="1" id="twofactorauth_retry" translate="label" type="text" sortOrder="40"
3939
showInDefault="1" showInWebsite="0" showInStore="0">
40-
<label>Configuration for 2FA retry attempts</label>
40+
<label>Configuration for TwoFactorAuth retry attempts</label>
4141
<comment>Security configurations for TwoFactorAuth page.</comment>
4242
</field>
4343
<field canRestore="1" id="auth_lock_expire" translate="label" type="text" sortOrder="40"
4444
showInDefault="1" showInWebsite="0" showInStore="0">
4545
<label>Configuration for TwoFactorAuth lock expire time</label>
46-
<comment>TwoFactor Authentication Configuration.</comment>
46+
<comment>TwoFactorAuth Configuration.</comment>
4747
</field>
4848
</group>
4949
<group id="google" translate="label" type="text" sortOrder="30" showInDefault="1" showInWebsite="0"

0 commit comments

Comments
 (0)