Skip to content

Commit 19bfa97

Browse files
pawan-adobe-securitypawan-adobe-security
committed
Merge branch 'pre-release' into cia-2.4.8-beta1-develop-bugfix-09032024
2 parents dae5657 + d3a759b commit 19bfa97

File tree

9 files changed

+201
-10
lines changed

9 files changed

+201
-10
lines changed

‎TwoFactorAuth/Controller/Adminhtml/Authy/Authpost.php

100644100755
+53-1
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,13 @@
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;
24+
use Magento\Framework\App\ObjectManager;
2225

2326
/**
2427
* @SuppressWarnings(PHPMD.CamelCaseMethodName)
28+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
2529
*/
2630
class Authpost extends AbstractAction implements HttpPostActionInterface
2731
{
@@ -60,6 +64,26 @@ class Authpost extends AbstractAction implements HttpPostActionInterface
6064
*/
6165
private $alert;
6266

67+
/**
68+
* Config path for the 2FA Attempts
69+
*/
70+
private const XML_PATH_2FA_RETRY_ATTEMPTS = 'twofactorauth/general/twofactorauth_retry';
71+
72+
/**
73+
* Config path for the 2FA Attempts
74+
*/
75+
private const XML_PATH_2FA_LOCK_EXPIRE = 'twofactorauth/general/auth_lock_expire';
76+
77+
/**
78+
* @var ScopeConfigInterface
79+
*/
80+
private $scopeConfig;
81+
82+
/**
83+
* @var UserResource
84+
*/
85+
private $userResource;
86+
6387
/**
6488
* @param Action\Context $context
6589
* @param Session $session
@@ -69,6 +93,9 @@ class Authpost extends AbstractAction implements HttpPostActionInterface
6993
* @param TfaInterface $tfa
7094
* @param AlertInterface $alert
7195
* @param DataObjectFactory $dataObjectFactory
96+
* @param UserResource|null $userResource
97+
* @param ScopeConfigInterface|null $scopeConfig
98+
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
7299
*/
73100
public function __construct(
74101
Action\Context $context,
@@ -78,7 +105,9 @@ public function __construct(
78105
TfaSessionInterface $tfaSession,
79106
TfaInterface $tfa,
80107
AlertInterface $alert,
81-
DataObjectFactory $dataObjectFactory
108+
DataObjectFactory $dataObjectFactory,
109+
?UserResource $userResource = null,
110+
?ScopeConfigInterface $scopeConfig = null
82111
) {
83112
parent::__construct($context);
84113
$this->tfa = $tfa;
@@ -88,6 +117,8 @@ public function __construct(
88117
$this->authy = $authy;
89118
$this->dataObjectFactory = $dataObjectFactory;
90119
$this->alert = $alert;
120+
$this->scopeConfig = $scopeConfig ?? ObjectManager::getInstance()->get(ScopeConfigInterface::class);
121+
$this->userResource = $userResource ?? ObjectManager::getInstance()->get(UserResource::class);
91122
}
92123

93124
/**
@@ -109,6 +140,13 @@ public function execute()
109140
$result = $this->jsonFactory->create();
110141

111142
try {
143+
if (!$this->allowApiRetries()) { //locked the user
144+
$lockThreshold = $this->scopeConfig->getValue(self::XML_PATH_2FA_LOCK_EXPIRE);
145+
if ($this->userResource->lock((int)$user->getId(), 0, $lockThreshold)) {
146+
$result->setData(['success' => false, 'message' => "Your account is temporarily disabled."]);
147+
return $result;
148+
}
149+
}
112150
$this->authy->verify($user, $this->dataObjectFactory->create([
113151
'data' => $this->getRequest()->getParams(),
114152
]));
@@ -141,4 +179,18 @@ protected function _isAllowed()
141179
$this->tfa->getProviderIsAllowed((int) $user->getId(), Authy::CODE) &&
142180
$this->tfa->getProvider(Authy::CODE)->isActive((int) $user->getId());
143181
}
182+
183+
/**
184+
* Check if retry attempt above threshold value
185+
*
186+
* @return bool
187+
*/
188+
private function allowApiRetries() : bool
189+
{
190+
$maxRetries = $this->scopeConfig->getValue(self::XML_PATH_2FA_RETRY_ATTEMPTS);
191+
$verifyAttempts = $this->session->getOtpAttempt();
192+
$verifyAttempts = $verifyAttempts === null ? 1 : $verifyAttempts+1;
193+
$this->session->setOtpAttempt($verifyAttempts);
194+
return $maxRetries >= $verifyAttempts;
195+
}
144196
}

‎TwoFactorAuth/Controller/Adminhtml/Google/Authpost.php

100644100755
+53-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
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;
24+
use Magento\Framework\App\ObjectManager;
2225

2326
/**
2427
* Google authenticator post controller
@@ -61,6 +64,26 @@ class Authpost extends AbstractAction implements HttpPostActionInterface
6164
*/
6265
private $alert;
6366

67+
/**
68+
* Config path for the 2FA Attempts
69+
*/
70+
private const XML_PATH_2FA_RETRY_ATTEMPTS = 'twofactorauth/general/twofactorauth_retry';
71+
72+
/**
73+
* Config path for the 2FA Attempts
74+
*/
75+
private const XML_PATH_2FA_LOCK_EXPIRE = 'twofactorauth/general/auth_lock_expire';
76+
77+
/**
78+
* @var ScopeConfigInterface
79+
*/
80+
private $scopeConfig;
81+
82+
/**
83+
* @var UserResource
84+
*/
85+
private $userResource;
86+
6487
/**
6588
* @param Action\Context $context
6689
* @param Session $session
@@ -70,6 +93,9 @@ class Authpost extends AbstractAction implements HttpPostActionInterface
7093
* @param TfaInterface $tfa
7194
* @param AlertInterface $alert
7295
* @param DataObjectFactory $dataObjectFactory
96+
* @param UserResource|null $userResource
97+
* @param ScopeConfigInterface|null $scopeConfig
98+
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
7399
*/
74100
public function __construct(
75101
Action\Context $context,
@@ -79,7 +105,9 @@ public function __construct(
79105
TfaSessionInterface $tfaSession,
80106
TfaInterface $tfa,
81107
AlertInterface $alert,
82-
DataObjectFactory $dataObjectFactory
108+
DataObjectFactory $dataObjectFactory,
109+
?UserResource $userResource = null,
110+
?ScopeConfigInterface $scopeConfig = null
83111
) {
84112
parent::__construct($context);
85113
$this->tfa = $tfa;
@@ -89,6 +117,8 @@ public function __construct(
89117
$this->tfaSession = $tfaSession;
90118
$this->dataObjectFactory = $dataObjectFactory;
91119
$this->alert = $alert;
120+
$this->scopeConfig = $scopeConfig ?? ObjectManager::getInstance()->get(ScopeConfigInterface::class);
121+
$this->userResource = $userResource ?? ObjectManager::getInstance()->get(UserResource::class);
92122
}
93123

94124
/**
@@ -103,6 +133,14 @@ public function execute()
103133
/** @var \Magento\Framework\DataObject $request */
104134
$request = $this->dataObjectFactory->create(['data' => $this->getRequest()->getParams()]);
105135

136+
if (!$this->allowApiRetries()) { //locked the user
137+
$lockThreshold = $this->scopeConfig->getValue(self::XML_PATH_2FA_LOCK_EXPIRE);
138+
if ($this->userResource->lock((int)$user->getId(), 0, $lockThreshold)) {
139+
$response->setData(['success' => false, 'message' => "Your account is temporarily disabled."]);
140+
return $response;
141+
}
142+
}
143+
106144
if ($this->google->verify($user, $request)) {
107145
$this->tfaSession->grantAccess();
108146
$response->setData(['success' => true]);
@@ -133,4 +171,18 @@ protected function _isAllowed()
133171
&& $this->tfa->getProviderIsAllowed((int)$user->getId(), Google::CODE)
134172
&& $this->tfa->getProvider(Google::CODE)->isActive((int)$user->getId());
135173
}
174+
175+
/**
176+
* Check if retry attempt above threshold value
177+
*
178+
* @return bool
179+
*/
180+
private function allowApiRetries() : bool
181+
{
182+
$maxRetries = $this->scopeConfig->getValue(self::XML_PATH_2FA_RETRY_ATTEMPTS);
183+
$verifyAttempts = $this->session->getOtpAttempt();
184+
$verifyAttempts = $verifyAttempts === null ? 1 : $verifyAttempts+1;
185+
$this->session->setOtpAttempt($verifyAttempts);
186+
return $maxRetries >= $verifyAttempts;
187+
}
136188
}

‎TwoFactorAuth/Model/Config/Backend/Leeway.php

+64
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
<?php
2+
/**
3+
* Copyright 2024 Adobe
4+
* All Rights Reserved.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\TwoFactorAuth\Model\Config\Backend;
9+
10+
use Magento\Framework\App\Config\Value;
11+
use Magento\Framework\App\Config\Data\ProcessorInterface;
12+
use Magento\Framework\Exception\ValidatorException;
13+
use OTPHP\TOTPInterface;
14+
15+
class Leeway extends Value implements ProcessorInterface
16+
{
17+
/**
18+
* Fetch Totp default period value
19+
*
20+
* @return int
21+
*/
22+
private function getDefaultPeriod(): int
23+
{
24+
return TOTPInterface::DEFAULT_PERIOD;
25+
}
26+
27+
/**
28+
* Process the value before saving.
29+
*
30+
* @param mixed $value The configuration value.
31+
* @return mixed The processed value.
32+
* @throws ValidatorException If the value is invalid.
33+
*/
34+
public function processValue($value)
35+
{
36+
if (!is_numeric($value)) {
37+
throw new ValidatorException(__('The Leeway must be a numeric value.'));
38+
}
39+
$numericValue = (int) $value;
40+
return $numericValue;
41+
}
42+
43+
/**
44+
* Validates the value before saving.
45+
*
46+
* @throws ValidatorException If the value is invalid.
47+
*/
48+
public function beforeSave()
49+
{
50+
$value = $this->getValue();
51+
$period = $this->getDefaultPeriod();
52+
if (!is_numeric($value) || $value < 1 || $value >= $period) {
53+
throw new ValidatorException(
54+
__(
55+
'Invalid Leeway value. It must be between 1 and %1 as default period is %2',
56+
$period-1,
57+
$period
58+
)
59+
);
60+
}
61+
62+
return parent::beforeSave();
63+
}
64+
}

‎TwoFactorAuth/Model/Provider/Engine/Google.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class Google implements EngineInterface
3535
/**
3636
* Config path for the OTP window
3737
*/
38-
const XML_PATH_OTP_WINDOW = 'twofactorauth/google/otp_window';
38+
public const XML_PATH_LEEWAY = 'twofactorauth/google/leeway';
3939

4040
/**
4141
* Engine code
@@ -199,7 +199,7 @@ public function verify(UserInterface $user, DataObject $request): bool
199199
return $totp->verify(
200200
$token,
201201
null,
202-
$config['window'] ?? (int)$this->scopeConfig->getValue(self::XML_PATH_OTP_WINDOW) ?: null
202+
$config['window'] ?? (int)$this->scopeConfig->getValue(self::XML_PATH_LEEWAY) ?: null
203203
);
204204
}
205205

‎TwoFactorAuth/Test/Api/GoogleActivateTest.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ public function testAlreadyActivatedProvider()
129129
/**
130130
* @magentoConfigFixture twofactorauth/general/force_providers google
131131
* @magentoApiDataFixture Magento/User/_files/user_with_custom_role.php
132-
* @magentoConfigFixture twofactorauth/google/otp_window 20
132+
* @magentoConfigFixture twofactorauth/google/leeway 29
133133
*/
134134
public function testActivate()
135135
{

‎TwoFactorAuth/Test/Api/GoogleAuthenticateTest.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ public function testNotConfiguredProvider(): void
223223
/**
224224
* @magentoConfigFixture twofactorauth/general/force_providers google
225225
* @magentoApiDataFixture Magento/User/_files/user_with_custom_role.php
226-
* @magentoConfigFixture twofactorauth/google/otp_window 20
226+
* @magentoConfigFixture twofactorauth/google/leeway 29
227227
*
228228
* @return void
229229
*/

‎TwoFactorAuth/etc/adminhtml/system.xml

100644100755
+19-3
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,30 @@
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>
3737
</field>
38+
<field canRestore="1" id="twofactorauth_retry" translate="label" type="text" sortOrder="40"
39+
showInDefault="1" showInWebsite="0" showInStore="0">
40+
<label>Retry attempt limit for Two-Factor Authentication</label>
41+
<comment>
42+
Determines how many times a user can try to complete the 2FA process before being temporarily locked out.
43+
</comment>
44+
</field>
45+
<field canRestore="1" id="auth_lock_expire" translate="label" type="text" sortOrder="40"
46+
showInDefault="1" showInWebsite="0" showInStore="0">
47+
<label>Two-Factor Authentication lockout time (seconds)</label>
48+
<comment>
49+
Determines how long a user is locked out if they reach the threshold defined above.
50+
Please enter at least 120 and at most 1800 (30 minutes).
51+
</comment>
52+
</field>
3853
</group>
3954
<group id="google" translate="label" type="text" sortOrder="30" showInDefault="1" showInWebsite="0"
4055
showInStore="0">
4156
<label>Google</label>
42-
<field id="otp_window" translate="label comment" type="text" sortOrder="10" showInDefault="1"
57+
<field id="leeway" translate="label comment" type="text" sortOrder="10" showInDefault="1"
4358
showInWebsite="0" showInStore="0" canRestore="1">
44-
<label>OTP Window</label>
45-
<comment>This determines how long the one-time-passwords are valid for. An OTP Window of 1 will result in the current OTP value plus 1 code in the past and 1 code in the future to be valid at any given point in time.</comment>
59+
<label>Leeway</label>
60+
<comment>This sets the time drift leeway for OTPs. A leeway of 29 with a period of 30 means OTPs are valid within ±29 seconds from the current time. The leeway must be smaller than the period</comment>
61+
<backend_model>Magento\TwoFactorAuth\Model\Config\Backend\Leeway</backend_model>
4662
</field>
4763
</group>
4864
<group id="duo" translate="label" type="text" sortOrder="40" showInDefault="1" showInWebsite="0"

‎TwoFactorAuth/etc/config.xml

+3-1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
<twofactorauth>
1212
<general>
1313
<force_providers></force_providers>
14+
<twofactorauth_retry>10</twofactorauth_retry>
15+
<auth_lock_expire>300</auth_lock_expire>
1416
</general>
1517
<authy>
1618
<onetouch_message>Login request to your Magento Admin</onetouch_message>
@@ -21,7 +23,7 @@
2123
<application_key backend_model="Magento\Config\Model\Config\Backend\Encrypted"/>
2224
</duo>
2325
<google>
24-
<otp_window>1</otp_window>
26+
<leeway backend_model="Magento\TwoFactorAuth\Model\Config\Backend\Leeway">29</leeway>
2527
</google>
2628
</twofactorauth>
2729
</default>

‎TwoFactorAuth/i18n/en_US.csv

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
"Configuration for TwoFactorAuth retry attempts","Configuration for TwoFactorAuth retry attempts"
2+
"Configuration for TwoFactorAuth lock expire time","Configuration for TwoFactorAuth lock expire time"
3+
"TwoFactorAuth Configuration","TwoFactorAuth Configuration"
4+
"Security configurations for TwoFactorAuth page","Security configurations for TwoFactorAuth page"
5+
"Your account is temporarily disabled.","Your account is temporarily disabled."

0 commit comments

Comments
 (0)