Skip to content

Commit a8f42f1

Browse files
Rizwan KhanRizwan Khan
authored andcommitted
AC-9797: 2FA functionality enhancement
1 parent c3c1edd commit a8f42f1

File tree

6 files changed

+126
-54
lines changed

6 files changed

+126
-54
lines changed

TwoFactorAuth/Controller/Adminhtml/Authy/Authpost.php

Lines changed: 56 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractAction;
2020
use Magento\TwoFactorAuth\Model\Provider\Engine\Authy;
2121
use Magento\User\Model\User;
22+
use Magento\Framework\App\Config\ScopeConfigInterface;
23+
use Magento\User\Model\ResourceModel\User as UserResource;
2224

2325
/**
2426
* @SuppressWarnings(PHPMD.CamelCaseMethodName)
@@ -60,6 +62,26 @@ class Authpost extends AbstractAction implements HttpPostActionInterface
6062
*/
6163
private $alert;
6264

65+
/**
66+
* Config path for the 2FA Attempts
67+
*/
68+
private const XML_PATH_2FA_RETRY_ATTEMPTS = 'twofactorauth/general/twofactorauth_retry';
69+
70+
/**
71+
* Config path for the 2FA Attempts
72+
*/
73+
private const XML_PATH_2FA_LOCK_EXPIRE = 'twofactorauth/general/auth_lock_expire';
74+
75+
/**
76+
* @var ScopeConfigInterface
77+
*/
78+
private $scopeConfig;
79+
80+
/**
81+
* @var UserResource
82+
*/
83+
protected $userResource;
84+
6385
/**
6486
* @param Action\Context $context
6587
* @param Session $session
@@ -69,6 +91,8 @@ class Authpost extends AbstractAction implements HttpPostActionInterface
6991
* @param TfaInterface $tfa
7092
* @param AlertInterface $alert
7193
* @param DataObjectFactory $dataObjectFactory
94+
* @param UserResource $userResource
95+
* @param ScopeConfigInterface $scopeConfig
7296
*/
7397
public function __construct(
7498
Action\Context $context,
@@ -78,7 +102,9 @@ public function __construct(
78102
TfaSessionInterface $tfaSession,
79103
TfaInterface $tfa,
80104
AlertInterface $alert,
81-
DataObjectFactory $dataObjectFactory
105+
DataObjectFactory $dataObjectFactory,
106+
UserResource $userResource,
107+
ScopeConfigInterface $scopeConfig
82108
) {
83109
parent::__construct($context);
84110
$this->tfa = $tfa;
@@ -88,6 +114,8 @@ public function __construct(
88114
$this->authy = $authy;
89115
$this->dataObjectFactory = $dataObjectFactory;
90116
$this->alert = $alert;
117+
$this->userResource = $userResource;
118+
$this->scopeConfig = $scopeConfig;
91119
}
92120

93121
/**
@@ -109,11 +137,20 @@ public function execute()
109137
$result = $this->jsonFactory->create();
110138

111139
try {
112-
$this->authy->verify($user, $this->dataObjectFactory->create([
113-
'data' => $this->getRequest()->getParams(),
114-
]));
115-
$this->tfaSession->grantAccess();
116-
$result->setData(['success' => true]);
140+
$maxRetries = $this->scopeConfig->getValue(self::XML_PATH_2FA_RETRY_ATTEMPTS);
141+
$retries = $this->verifyRetryAttempts();
142+
if ($retries > $maxRetries) { //locked the user
143+
$lockThreshold = $this->scopeConfig->getValue(self::XML_PATH_2FA_LOCK_EXPIRE);
144+
if ($this->userResource->lock($user->getId(),0, $lockThreshold)) {
145+
$result->setData(['success' => false, 'message' => "User is disabled temporarily!"]);
146+
}
147+
} else {
148+
$this->authy->verify($user, $this->dataObjectFactory->create([
149+
'data' => $this->getRequest()->getParams(),
150+
]));
151+
$this->tfaSession->grantAccess();
152+
$result->setData(['success' => true]);
153+
}
117154
} catch (Exception $e) {
118155
$this->alert->event(
119156
'Magento_TwoFactorAuth',
@@ -141,4 +178,17 @@ protected function _isAllowed()
141178
$this->tfa->getProviderIsAllowed((int) $user->getId(), Authy::CODE) &&
142179
$this->tfa->getProvider(Authy::CODE)->isActive((int) $user->getId());
143180
}
181+
182+
/**
183+
* Get retry attempt count
184+
*
185+
* @return int
186+
*/
187+
private function verifyRetryAttempts() : int
188+
{
189+
$verifyAttempts = $this->session->getOtpAttempt();
190+
$verifyAttempts = is_null($verifyAttempts) ? 0 : $verifyAttempts+1;
191+
$this->session->setOtpAttempt($verifyAttempts);
192+
return $verifyAttempts;
193+
}
144194
}

TwoFactorAuth/Controller/Adminhtml/Google/Authpost.php

Lines changed: 62 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractAction;
2020
use Magento\TwoFactorAuth\Model\Provider\Engine\Google;
2121
use Magento\User\Model\User;
22+
use Magento\Framework\App\Config\ScopeConfigInterface;
23+
use Magento\User\Model\ResourceModel\User as UserResource;
2224

2325
/**
2426
* Google authenticator post controller
@@ -61,6 +63,26 @@ class Authpost extends AbstractAction implements HttpPostActionInterface
6163
*/
6264
private $alert;
6365

66+
/**
67+
* Config path for the 2FA Attempts
68+
*/
69+
private const XML_PATH_2FA_RETRY_ATTEMPTS = 'twofactorauth/general/twofactorauth_retry';
70+
71+
/**
72+
* Config path for the 2FA Attempts
73+
*/
74+
private const XML_PATH_2FA_LOCK_EXPIRE = 'twofactorauth/general/auth_lock_expire';
75+
76+
/**
77+
* @var ScopeConfigInterface
78+
*/
79+
private $scopeConfig;
80+
81+
/**
82+
* @var UserResource
83+
*/
84+
protected $userResource;
85+
6486
/**
6587
* @param Action\Context $context
6688
* @param Session $session
@@ -70,6 +92,8 @@ class Authpost extends AbstractAction implements HttpPostActionInterface
7092
* @param TfaInterface $tfa
7193
* @param AlertInterface $alert
7294
* @param DataObjectFactory $dataObjectFactory
95+
* @param UserResource $userResource
96+
* @param ScopeConfigInterface $scopeConfig
7397
*/
7498
public function __construct(
7599
Action\Context $context,
@@ -79,7 +103,9 @@ public function __construct(
79103
TfaSessionInterface $tfaSession,
80104
TfaInterface $tfa,
81105
AlertInterface $alert,
82-
DataObjectFactory $dataObjectFactory
106+
DataObjectFactory $dataObjectFactory,
107+
UserResource $userResource,
108+
ScopeConfigInterface $scopeConfig
83109
) {
84110
parent::__construct($context);
85111
$this->tfa = $tfa;
@@ -89,6 +115,8 @@ public function __construct(
89115
$this->tfaSession = $tfaSession;
90116
$this->dataObjectFactory = $dataObjectFactory;
91117
$this->alert = $alert;
118+
$this->userResource = $userResource;
119+
$this->scopeConfig = $scopeConfig;
92120
}
93121

94122
/**
@@ -103,18 +131,27 @@ public function execute()
103131
/** @var \Magento\Framework\DataObject $request */
104132
$request = $this->dataObjectFactory->create(['data' => $this->getRequest()->getParams()]);
105133

106-
if ($this->google->verify($user, $request)) {
107-
$this->tfaSession->grantAccess();
108-
$response->setData(['success' => true]);
134+
$maxRetries = $this->scopeConfig->getValue(self::XML_PATH_2FA_RETRY_ATTEMPTS);
135+
$retries = $this->verifyRetryAttempts();
136+
if ($retries > $maxRetries) { //locked the user
137+
$lockThreshold = $this->scopeConfig->getValue(self::XML_PATH_2FA_LOCK_EXPIRE);
138+
if ($this->userResource->lock($user->getId(),0, $lockThreshold)) {
139+
$response->setData(['success' => false, 'message' => "User is disabled temporarily!"]);
140+
}
109141
} else {
110-
$this->alert->event(
111-
'Magento_TwoFactorAuth',
112-
'Google auth invalid token',
113-
AlertInterface::LEVEL_WARNING,
114-
$user->getUserName()
115-
);
116-
117-
$response->setData(['success' => false, 'message' => 'Invalid code']);
142+
if ($this->google->verify($user, $request)) {
143+
$this->tfaSession->grantAccess();
144+
$response->setData(['success' => true]);
145+
} else {
146+
$this->alert->event(
147+
'Magento_TwoFactorAuth',
148+
'Google auth invalid token',
149+
AlertInterface::LEVEL_WARNING,
150+
$user->getUserName()
151+
);
152+
153+
$response->setData(['success' => false, 'message' => 'Invalid code']);
154+
}
118155
}
119156

120157
return $response;
@@ -133,4 +170,17 @@ protected function _isAllowed()
133170
&& $this->tfa->getProviderIsAllowed((int)$user->getId(), Google::CODE)
134171
&& $this->tfa->getProvider(Google::CODE)->isActive((int)$user->getId());
135172
}
173+
174+
/**
175+
* Get retry attempt count
176+
*
177+
* @return int
178+
*/
179+
private function verifyRetryAttempts() : int
180+
{
181+
$verifyAttempts = $this->session->getOtpAttempt();
182+
$verifyAttempts = is_null($verifyAttempts) ? 0 : $verifyAttempts+1;
183+
$this->session->setOtpAttempt($verifyAttempts);
184+
return $verifyAttempts;
185+
}
136186
}

TwoFactorAuth/etc/adminhtml/system.xml

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,16 @@
3434
showInDefault="1" showInWebsite="0" showInStore="0">
3535
<label>Configuration Email URL for Web API</label>
3636
<comment>This can be used to override the default email configuration link that is sent when using the Magento Web API's to authenticate. Use the placeholder :tfat to indicate where the token should be injected</comment>
37-
</field>
37+
</field>x
3838
<field canRestore="1" id="twofactorauth_retry" translate="label" type="text" sortOrder="40"
3939
showInDefault="1" showInWebsite="0" showInStore="0">
4040
<label>Configuration for 2FA retry attempts</label>
41-
<comment>Security configurations for TowFatcorAuth page. To avoid the bruteforce of twofactorauth API</comment>
41+
<comment>Security configurations for TowFatcorAuth page.</comment>
42+
</field>
43+
<field canRestore="1" id="auth_lock_expire" translate="label" type="text" sortOrder="40"
44+
showInDefault="1" showInWebsite="0" showInStore="0">
45+
<label>Configuration for 2FA lock expire time</label>
46+
<comment>TwoFactor Authentation Configuration.</comment>
4247
</field>
4348
</group>
4449
<group id="google" translate="label" type="text" sortOrder="30" showInDefault="1" showInWebsite="0"

TwoFactorAuth/etc/config.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
<general>
1313
<force_providers></force_providers>
1414
<twofactorauth_retry>10</twofactorauth_retry>
15+
<auth_lock_expire>300</auth_lock_expire>
1516
</general>
1617
<authy>
1718
<onetouch_message>Login request to your Magento Admin</onetouch_message>

TwoFactorAuth/view/adminhtml/web/js/authy/auth.js

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ define([
1212
], function ($, ko, Component, error) {
1313
'use strict';
1414

15-
let attempts = 0;
16-
1715
return Component.extend({
1816
selectedMethod: ko.observable(''),
1917
waitingText: ko.observable(''),
@@ -192,27 +190,12 @@ define([
192190
this.success(false);
193191
},
194192

195-
/**
196-
* Get Retry Attempts
197-
* @returns {int}
198-
*/
199-
getRetryAttempts: function () {
200-
return this.attempts;
201-
},
202-
203193
/**
204194
* Verify authy code
205195
*/
206196
verifyCode: function () {
207197
var me = this;
208198

209-
attempts++;
210-
if (attempts > this.getRetryAttempts()) {
211-
console.log('Maximum otp retries are done.');
212-
location.href = $('.tfa-logout-link').attr('href');
213-
return;
214-
}
215-
216199
this.waitingText('Please wait...');
217200

218201
$.post(this.getPostUrl(), {

TwoFactorAuth/view/adminhtml/web/js/google/auth.js

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@ define([
1111
], function ($, ko, Component, error) {
1212
'use strict';
1313

14-
let attempts = 0;
15-
1614
return Component.extend({
1715
currentStep: ko.observable('register'),
1816
waitText: ko.observable(''),
@@ -42,14 +40,6 @@ define([
4240
return this.postUrl;
4341
},
4442

45-
/**
46-
* Get Retry Attempts
47-
* @returns {int}
48-
*/
49-
getRetryAttempts: function () {
50-
return this.attempts;
51-
},
52-
5343
/**
5444
* Get plain Secret Code
5545
* @returns {String}
@@ -72,13 +62,6 @@ define([
7262
doVerify: function () {
7363
var me = this;
7464

75-
attempts++;
76-
if (attempts > this.getRetryAttempts()) {
77-
console.log('Maximum otp retries are done.');
78-
location.href = $('.tfa-logout-link').attr('href');
79-
return;
80-
}
81-
8265
this.waitText('Please wait...');
8366
$.post(this.getPostUrl(), {
8467
'tfa_code': this.verifyCode()

0 commit comments

Comments
 (0)