MC-22950: Enable 2FA by default for Admins

nathanjosiah · nathanjosiah · commit 19c76d2650fc · 2020-03-25T15:48:31.000-05:00
- Removed redundant code
- Fixed corner case with skipping configuration
- fixed tests that don't cleanup
diff --git a/TwoFactorAuth/Controller/Adminhtml/Tfa/Index.php b/TwoFactorAuth/Controller/Adminhtml/Tfa/Index.php
@@ -118,7 +118,9 @@ public function execute()
         $providerCode = '';
 
         $defaultProviderCode = $this->userConfigManager->getDefaultProvider((int) $user->getId());
-        if ($this->tfa->getProviderIsAllowed((int) $user->getId(), $defaultProviderCode)) {
+        if ($this->tfa->getProviderIsAllowed((int) $user->getId(), $defaultProviderCode)
+            && $this->tfa->getProvider($defaultProviderCode)->isActive((int) $user->getId())
+        ) {
             //If default provider was configured - select it.
             $providerCode = $defaultProviderCode;
         }
@@ -128,8 +130,16 @@ public function execute()
             $providers = $this->tfa->getUserProviders((int) $user->getId());
             if (!empty($providers)) {
                 foreach ($providers as $enabledProvider) {
-                    $providerCode = $enabledProvider->getCode();
-                    break;
+                    /*
+                     * The user has skipped all providers that need to be configured but there is
+                     * also at least one already configured
+                     */
+                    if (!in_array($enabledProvider->getCode(), $currentlySkipped)
+                        && !in_array($enabledProvider->getCode(), $toActivateCodes)
+                    ) {
+                        $providerCode = $enabledProvider->getCode();
+                        break;
+                    }
                 }
             }
         }
diff --git a/TwoFactorAuth/Test/Integration/Block/ChangeProviderTest.php b/TwoFactorAuth/Test/Integration/Block/ChangeProviderTest.php
@@ -20,6 +20,9 @@
 use Magento\TwoFactorAuth\Model\Provider\Engine\Google;
 use PHPUnit\Framework\TestCase;
 
+/**
+ * @magentoDbIsolation enabled
+ */
 class ChangeProviderTest extends TestCase
 {
     /**
diff --git a/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Authy/ConfigureTest.php b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Authy/ConfigureTest.php
@@ -10,6 +10,7 @@
  * Test for the configure authy 2FA form page.
  *
  * @magentoAppArea adminhtml
+ * @magentoDbIsolation enabled
  */
 class ConfigureTest extends AbstractConfigureBackendController
 {
diff --git a/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Authy/ConfigurepostTest.php b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Authy/ConfigurepostTest.php
@@ -10,6 +10,7 @@
  * Test for the configure authy 2FA form processor.
  *
  * @magentoAppArea adminhtml
+ * @magentoDbIsolation enabled
  */
 class ConfigurepostTest extends AbstractConfigureBackendController
 {
diff --git a/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Authy/ConfigureverifypostTest.php b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Authy/ConfigureverifypostTest.php
@@ -10,6 +10,7 @@
  * Test for the configure authy 2FA form processor.
  *
  * @magentoAppArea adminhtml
+ * @magentoDbIsolation enabled
  */
 class ConfigureverifypostTest extends AbstractConfigureBackendController
 {
diff --git a/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Duo/AuthTest.php b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Duo/AuthTest.php
@@ -10,6 +10,7 @@
  * Test for the DuoSecurity form.
  *
  * @magentoAppArea adminhtml
+ * @magentoDbIsolation enabled
  */
 class AuthTest extends AbstractConfigureBackendController
 {
diff --git a/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Duo/AuthpostTest.php b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Duo/AuthpostTest.php
@@ -10,6 +10,7 @@
  * Test for the DuoSecurity form processor.
  *
  * @magentoAppArea adminhtml
+ * @magentoDbIsolation enabled
  */
 class AuthpostTest extends AbstractConfigureBackendController
 {
diff --git a/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Google/ConfigureTest.php b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Google/ConfigureTest.php
@@ -10,6 +10,7 @@
  * Test for the configure google 2FA form page.
  *
  * @magentoAppArea adminhtml
+ * @magentoDbIsolation enabled
  */
 class ConfigureTest extends AbstractConfigureBackendController
 {
diff --git a/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Google/ConfigurepostTest.php b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Google/ConfigurepostTest.php
@@ -10,6 +10,7 @@
  * Test for the configure google 2FA form page processor.
  *
  * @magentoAppArea adminhtml
+ * @magentoDbIsolation enabled
  */
 class ConfigurepostTest extends AbstractConfigureBackendController
 {
diff --git a/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Tfa/ConfigureTest.php b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Tfa/ConfigureTest.php
@@ -12,6 +12,7 @@
  * Testing the controller for the page that presents forced providers list to users.
  *
  * @magentoAppArea adminhtml
+ * @magentoDbIsolation enabled
  */
 class ConfigureTest extends AbstractBackendController
 {
diff --git a/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Tfa/IndexTest.php b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Tfa/IndexTest.php
@@ -15,6 +15,7 @@
  * Testing the controller for the page that redirects user to proper pages depending on 2FA state.
  *
  * @magentoAppArea adminhtml
+ * @magentoDbIsolation enabled
  */
 class IndexTest extends AbstractBackendController
 {
diff --git a/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Tfa/RequestconfigTest.php b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Tfa/RequestconfigTest.php
@@ -14,6 +14,7 @@
  * Testing the controller for the page that requests 2FA config from users.
  *
  * @magentoAppArea adminhtml
+ * @magentoDbIsolation enabled
  */
 class RequestconfigTest extends AbstractBackendController
 {
diff --git a/TwoFactorAuth/Test/Integration/Controller/Adminhtml/U2f/ConfigureTest.php b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/U2f/ConfigureTest.php
@@ -10,6 +10,7 @@
  * Test for the configure U2F 2FA form page.
  *
  * @magentoAppArea adminhtml
+ * @magentoDbIsolation enabled
  */
 class ConfigureTest extends AbstractConfigureBackendController
 {
diff --git a/TwoFactorAuth/Test/Integration/Controller/Adminhtml/U2f/ConfigurepostTest.php b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/U2f/ConfigurepostTest.php
@@ -10,6 +10,7 @@
  * Test for the configure U2F 2FA form processor.
  *
  * @magentoAppArea adminhtml
+ * @magentoDbIsolation enabled
  */
 class ConfigurepostTest extends AbstractConfigureBackendController
 {
diff --git a/TwoFactorAuth/Test/Integration/ControllerActionPredispatchTest.php b/TwoFactorAuth/Test/Integration/ControllerActionPredispatchTest.php
@@ -15,6 +15,7 @@
  * Test for 2FA enforcement.
  *
  * @magentoAppArea adminhtml
+ * @magentoDbIsolation enabled
  */
 class ControllerActionPredispatchTest extends AbstractBackendController
 {
diff --git a/TwoFactorAuth/Test/Integration/UserConfigManagerTest.php b/TwoFactorAuth/Test/Integration/UserConfigManagerTest.php
@@ -15,6 +15,9 @@
 use Magento\TwoFactorAuth\Api\UserConfigManagerInterface;
 use PHPUnit\Framework\TestCase;
 
+/**
+ * @magentoDbIsolation enabled
+ */
 class UserConfigManagerTest extends TestCase
 {
     /**
diff --git a/TwoFactorAuth/Test/Integration/UserConfigRequestManagerTest.php b/TwoFactorAuth/Test/Integration/UserConfigRequestManagerTest.php
@@ -13,6 +13,9 @@
 use Magento\TwoFactorAuth\Model\Provider\Engine\Google;
 use PHPUnit\Framework\TestCase;
 
+/**
+ * @magentoDbIsolation enabled
+ */
 class UserConfigRequestManagerTest extends TestCase
 {
     /**
diff --git a/TwoFactorAuth/Test/Integration/UserConfigTokenManagerTest.php b/TwoFactorAuth/Test/Integration/UserConfigTokenManagerTest.php
@@ -10,6 +10,9 @@
 use Magento\TestFramework\Helper\Bootstrap;
 use Magento\User\Model\UserFactory;
 
+/**
+ * @magentoDbIsolation enabled
+ */
 class UserConfigTokenManagerTest extends TestCase
 {
     /**
diff --git a/TwoFactorAuth/view/adminhtml/web/js/u2fkey/auth.js b/TwoFactorAuth/view/adminhtml/web/js/u2fkey/auth.js
@@ -145,32 +145,6 @@ define([
             }.bind(this));
         },
 
-        oldwaitForTouch: function () {
-            var requestData = this.authenticateData,
-                me = this;
-
-            // eslint-disable-next-line no-undef
-            u2f.sign(
-                requestData,
-                function (signResponse) {
-                    $.post(me.getPostUrl(), {
-                        'request': requestData,
-                        'response': signResponse
-                    }).done(function (res) {
-                        if (res.success) {
-                            me.currentStep('login');
-                            self.location.href = me.getSuccessUrl();
-                        } else {
-                            me.waitForTouch();
-                        }
-                    }).fail(function () {
-                        error.display('Invalid device');
-                        this.idle(true);
-                    });
-                }, 120
-            );
-        },
-
         /**
          * Handle WebAuthn failure
          *

Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,9 @@`
`20`	`20`	`use Magento\TwoFactorAuth\Model\Provider\Engine\Google;`
`21`	`21`	`use PHPUnit\Framework\TestCase;`
`22`	`22`
	`23`	`+/**`
	`24`	`+ * @magentoDbIsolation enabled`
	`25`	`+ */`
`23`	`26`	`class ChangeProviderTest extends TestCase`
`24`	`27`	`{`
`25`	`28`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -10,6 +10,7 @@`
`10`	`10`	`* Test for the configure authy 2FA form page.`
`11`	`11`	`*`
`12`	`12`	`* @magentoAppArea adminhtml`
	`13`	`+ * @magentoDbIsolation enabled`
`13`	`14`	`*/`
`14`	`15`	`class ConfigureTest extends AbstractConfigureBackendController`
`15`	`16`	`{`
Original file line number	Diff line number	Diff line change
`@@ -12,6 +12,7 @@`
`12`	`12`	`* Testing the controller for the page that presents forced providers list to users.`
`13`	`13`	`*`
`14`	`14`	`* @magentoAppArea adminhtml`
	`15`	`+ * @magentoDbIsolation enabled`
`15`	`16`	`*/`
`16`	`17`	`class ConfigureTest extends AbstractBackendController`
`17`	`18`	`{`