AC-9797: 2FA functionality enhancement

Author: Rizwan Khan

TwoFactorAuth/Controller/Adminhtml/Authy/Authpost.php

@@ -138,20 +138,18 @@ public function execute()
         $result = $this->jsonFactory->create();
 
         try {
-            $maxRetries = $this->scopeConfig->getValue(self::XML_PATH_2FA_RETRY_ATTEMPTS);
-            $retries = $this->verifyRetryAttempts();
-            if ($retries > $maxRetries) { //locked the user
+            if (!$this->allowApiRetries()) { //locked the user
                 $lockThreshold = $this->scopeConfig->getValue(self::XML_PATH_2FA_LOCK_EXPIRE);
                 if ($this->userResource->lock($user->getId(), 0, $lockThreshold)) {
                     $result->setData(['success' => false, 'message' => "User is disabled temporarily!"]);
+                    return $result;
                 }
-            } else {
-                $this->authy->verify($user, $this->dataObjectFactory->create([
-                    'data' => $this->getRequest()->getParams(),
-                ]));
-                $this->tfaSession->grantAccess();
-                $result->setData(['success' => true]);
             }
+            $this->authy->verify($user, $this->dataObjectFactory->create([
+                'data' => $this->getRequest()->getParams(),
+            ]));
+            $this->tfaSession->grantAccess();
+            $result->setData(['success' => true]);
         } catch (Exception $e) {
             $this->alert->event(
                 'Magento_TwoFactorAuth',
@@ -181,15 +179,20 @@ protected function _isAllowed()
     }
 
     /**
-     * Get retry attempt count
+     * Check if retry attempt above threshold value
      *
-     * @return int
+     * @return bool
      */
-    private function verifyRetryAttempts() : int
+    private function allowApiRetries() : bool
     {
+        $maxRetries = $this->scopeConfig->getValue(self::XML_PATH_2FA_RETRY_ATTEMPTS);
         $verifyAttempts = $this->session->getOtpAttempt();
         $verifyAttempts = $verifyAttempts === null ? 1 : $verifyAttempts+1;
         $this->session->setOtpAttempt($verifyAttempts);
-        return $verifyAttempts;
+        if ($verifyAttempts > $maxRetries) {
+            return false;
+        }
+
+        return true;
     }
 }

TwoFactorAuth/Controller/Adminhtml/Google/Authpost.php

@@ -13,6 +13,7 @@
 use Magento\Framework\Controller\Result\JsonFactory;
 use Magento\Framework\DataObjectFactory;
 use Magento\Framework\Exception\NoSuchEntityException;
+use Magento\Tests\NamingConvention\true\bool;
 use Magento\TwoFactorAuth\Model\AlertInterface;
 use Magento\TwoFactorAuth\Api\TfaInterface;
 use Magento\TwoFactorAuth\Api\TfaSessionInterface;
@@ -132,27 +133,26 @@ public function execute()
         /** @var \Magento\Framework\DataObject $request */
         $request = $this->dataObjectFactory->create(['data' => $this->getRequest()->getParams()]);
 
-        $maxRetries = $this->scopeConfig->getValue(self::XML_PATH_2FA_RETRY_ATTEMPTS);
-        $retries = $this->verifyRetryAttempts();
-        if ($retries > $maxRetries) { //locked the user
+        if (!$this->allowApiRetries()) { //locked the user
             $lockThreshold = $this->scopeConfig->getValue(self::XML_PATH_2FA_LOCK_EXPIRE);
             if ($this->userResource->lock($user->getId(), 0, $lockThreshold)) {
                 $response->setData(['success' => false, 'message' => "User is disabled temporarily!"]);
+                return $response;
             }
+        }
+
+        if ($this->google->verify($user, $request)) {
+            $this->tfaSession->grantAccess();
+            $response->setData(['success' => true]);
         } else {
-            if ($this->google->verify($user, $request)) {
-                $this->tfaSession->grantAccess();
-                $response->setData(['success' => true]);
-            } else {
-                $this->alert->event(
-                    'Magento_TwoFactorAuth',
-                    'Google auth invalid token',
-                    AlertInterface::LEVEL_WARNING,
-                    $user->getUserName()
-                );
-
-                $response->setData(['success' => false, 'message' => 'Invalid code']);
-            }
+            $this->alert->event(
+                'Magento_TwoFactorAuth',
+                'Google auth invalid token',
+                AlertInterface::LEVEL_WARNING,
+                $user->getUserName()
+            );
+
+            $response->setData(['success' => false, 'message' => 'Invalid code']);
         }
 
         return $response;
@@ -173,15 +173,20 @@ protected function _isAllowed()
     }
 
     /**
-     * Get retry attempt count
+     * Check if retry attempt above threshold value
      *
-     * @return int
+     * @return bool
      */
-    private function verifyRetryAttempts() : int
+    private function allowApiRetries() : bool
     {
+        $maxRetries = $this->scopeConfig->getValue(self::XML_PATH_2FA_RETRY_ATTEMPTS);
         $verifyAttempts = $this->session->getOtpAttempt();
         $verifyAttempts = $verifyAttempts === null ? 1 : $verifyAttempts+1;
         $this->session->setOtpAttempt($verifyAttempts);
-        return $verifyAttempts;
+        if ($verifyAttempts > $maxRetries) {
+            return false;
+        }
+
+        return true;
     }
 }

TwoFactorAuth/etc/adminhtml/system.xml

@@ -38,12 +38,12 @@
                 <field canRestore="1" id="twofactorauth_retry" translate="label" type="text" sortOrder="40"
                        showInDefault="1" showInWebsite="0" showInStore="0">
                     <label>Configuration for 2FA retry attempts</label>
-                    <comment>Security configurations for TowFatcorAuth page.</comment>
+                    <comment>Security configurations for TwoFactorAuth page.</comment>
                 </field>
                 <field canRestore="1" id="auth_lock_expire" translate="label" type="text" sortOrder="40"
                        showInDefault="1" showInWebsite="0" showInStore="0">
-                    <label>Configuration for 2FA lock expire time</label>
-                    <comment>TwoFactor Authentation Configuration.</comment>
+                    <label>Configuration for TwoFactorAuth lock expire time</label>
+                    <comment>TwoFactor Authentication Configuration.</comment>
                 </field>
             </group>
             <group id="google" translate="label" type="text" sortOrder="30" showInDefault="1" showInWebsite="0"