17352 include a warning to users about to turn on require 2fa (#17400)

* 17352 require user to set up 2fa before being able to turn on 'require 2fa'

* 17352 disable instead of hiding 'require 2fa' setting if user isnt using 2fa

8 files changed, 46 insertions, 34 deletions

diff --git a/plugins/TwoFactorAuth/Controller.php b/plugins/TwoFactorAuth/Controller.php
index cb9dedf7ab..6d24b9f9c2 100644
--- a/plugins/TwoFactorAuth/Controller.php
+++ b/plugins/TwoFactorAuth/Controller.php
@@ -122,7 +122,7 @@ class Controller extends \Piwik\Plugin\Controller
         $this->validator->checkCanUseTwoFa();
 
         return $this->renderTemplate('userSettings', array(
-            'isEnabled' => $this->twoFa->isUserUsingTwoFactorAuthentication(Piwik::getCurrentUserLogin()),
+            'isEnabled' => TwoFactorAuthentication::isUserUsingTwoFactorAuthentication(Piwik::getCurrentUserLogin()),
             'isForced' => $this->twoFa->isUserRequiredToHaveTwoFactorEnabled(),
             'disableNonce' => Nonce::getNonce(self::DISABLE_2FA_NONCE)
         ));
@@ -241,7 +241,7 @@ class Controller extends \Piwik\Plugin\Controller
         }
 
         if (!$this->recoveryCodeDao->getAllRecoveryCodesForLogin($login)
-            || (!$hasSubmittedForm && !$this->twoFa->isUserUsingTwoFactorAuthentication($login))) {
+            || (!$hasSubmittedForm && !TwoFactorAuthentication::isUserUsingTwoFactorAuthentication($login))) {
             // we cannot generate new codes after form has been submitted and user is not yet using 2fa cause we would
             // change recovery codes in the background without the user noticing... we cannot simply do this:
             // if !getAllRecoveryCodesForLogin => createRecoveryCodesForLogin. Because it could be a security issue that
@@ -254,7 +254,7 @@ class Controller extends \Piwik\Plugin\Controller
         $view->description = $login;
         $view->authCodeNonce = Nonce::getNonce(self::AUTH_CODE_NONCE);
         $view->AccessErrorString = $accessErrorString;
-        $view->isAlreadyUsing2fa = $this->twoFa->isUserUsingTwoFactorAuthentication($login);
+        $view->isAlreadyUsing2fa = TwoFactorAuthentication::isUserUsingTwoFactorAuthentication($login);
         $view->newSecret = $secret;
         $view->twoFaBarCodeSetupUrl = $this->getTwoFaBarCodeSetupUrl($secret);
         $view->codes = $this->recoveryCodeDao->getAllRecoveryCodesForLogin($login);
diff --git a/plugins/TwoFactorAuth/SystemSettings.php b/plugins/TwoFactorAuth/SystemSettings.php
index ee97f27c63..1fd6619ee9 100644
--- a/plugins/TwoFactorAuth/SystemSettings.php
+++ b/plugins/TwoFactorAuth/SystemSettings.php
@@ -8,10 +8,13 @@
 
 namespace Piwik\Plugins\TwoFactorAuth;
 
+use Piwik\Piwik;
 use Piwik\Plugin;
 use Piwik\Settings\Setting;
 use Piwik\Settings\FieldConfig;
 use Piwik\Url;
+use Piwik\Plugins\TwoFactorAuth\TwoFactorAuthentication;
+use Piwik\Container\StaticContainer;
 
 class SystemSettings extends \Piwik\Settings\Plugin\SystemSettings
 {
@@ -29,11 +32,18 @@ class SystemSettings extends \Piwik\Settings\Plugin\SystemSettings
 
     private function createRequire2FA()
     {
-        return $this->makeSetting('twoFactorAuthRequired', $default = false, FieldConfig::TYPE_BOOL, function (FieldConfig $field) {
-            $field->title = 'Require two-factor authentication for everyone';
-            $field->description = 'When enabled, every user has to enable two factor authentication.';
+        $setting = $this->makeSetting('twoFactorAuthRequired', $default = false, FieldConfig::TYPE_BOOL, function (FieldConfig $field) {
+            $field->title = Piwik::translate('TwoFactorAuth_RequireTwoFAForAll');
+            $field->description = Piwik::translate('TwoFactorAuth_RequireTwoFAForAllInformation');
             $field->uiControl = FieldConfig::UI_CONTROL_CHECKBOX;
+
+            $isWritable = defined('PIWIK_TEST_MODE') || TwoFactorAuthentication::isUserUsingTwoFactorAuthentication(Piwik::getCurrentUserLogin());
+            if (!$isWritable) {
+                $field->uiControlAttributes = ['disabled' => 'disabled'];
+            }
         });
+
+        return $setting;
     }
 
     private function create2FATitle()
diff --git a/plugins/TwoFactorAuth/TwoFactorAuth.php b/plugins/TwoFactorAuth/TwoFactorAuth.php
index 60f354ed23..8305c45411 100644
--- a/plugins/TwoFactorAuth/TwoFactorAuth.php
+++ b/plugins/TwoFactorAuth/TwoFactorAuth.php
@@ -82,7 +82,7 @@ class TwoFactorAuth extends \Piwik\Plugin
             $twoFa = $this->getTwoFa();
 
             if ($authCode
-                && $twoFa->isUserUsingTwoFactorAuthentication($login)
+                && TwoFactorAuthentication::isUserUsingTwoFactorAuthentication($login)
                 && $twoFa->validateAuthCode($login, $authCode)) {
                 $sessionFingerprint = new SessionFingerprint();
                 $sessionFingerprint->setTwoFactorAuthenticationVerified();
@@ -117,7 +117,7 @@ class TwoFactorAuth extends \Piwik\Plugin
             $login = $params['parameters']['userLogin'];
             $twoFa = $this->getTwoFa();
 
-            if ($twoFa->isUserUsingTwoFactorAuthentication($login) && $this->isValidTokenAuth($returnedValue)) {
+            if (TwoFactorAuthentication::isUserUsingTwoFactorAuthentication($login) && $this->isValidTokenAuth($returnedValue)) {
                 $authCode = Common::getRequestVar('authCode', '', 'string');
                 // we only return an error when the login/password combo was correct. otherwise you could brute force
                 // auth tokens
@@ -130,7 +130,7 @@ class TwoFactorAuth extends \Piwik\Plugin
                     throw new Exception(Piwik::translate('TwoFactorAuth_InvalidAuthCode'));
                 }
             } else if ($twoFa->isUserRequiredToHaveTwoFactorEnabled()
-                        && !$twoFa->isUserUsingTwoFactorAuthentication($login)) {
+                        && !TwoFactorAuthentication::isUserUsingTwoFactorAuthentication($login)) {
                 throw new Exception(Piwik::translate('TwoFactorAuth_RequiredAuthCodeNotConfiguredAPI'));
             }
         }
@@ -153,7 +153,7 @@ class TwoFactorAuth extends \Piwik\Plugin
 
         $twoFa = $this->getTwoFa();
 
-        $isUsing2FA = $twoFa->isUserUsingTwoFactorAuthentication(Piwik::getCurrentUserLogin());
+        $isUsing2FA = TwoFactorAuthentication::isUserUsingTwoFactorAuthentication(Piwik::getCurrentUserLogin());
         if ($isUsing2FA && !Request::isRootRequestApiRequest() && Session::isStarted()) {
             $sessionFingerprint = new SessionFingerprint();
             if (!$sessionFingerprint->hasVerifiedTwoFactor()) {
@@ -206,7 +206,7 @@ class TwoFactorAuth extends \Piwik\Plugin
 
         $twoFa = $this->getTwoFa();
 
-        $isUsing2FA = $twoFa->isUserUsingTwoFactorAuthentication(Piwik::getCurrentUserLogin());
+        $isUsing2FA = TwoFactorAuthentication::isUserUsingTwoFactorAuthentication(Piwik::getCurrentUserLogin());
         if ($isUsing2FA && !Request::isRootRequestApiRequest()) {
             $sessionFingerprint = new SessionFingerprint();
             if (!$sessionFingerprint->hasVerifiedTwoFactor()) {
diff --git a/plugins/TwoFactorAuth/TwoFactorAuthentication.php b/plugins/TwoFactorAuth/TwoFactorAuthentication.php
index f05769952e..6139e62dd7 100644
--- a/plugins/TwoFactorAuth/TwoFactorAuthentication.php
+++ b/plugins/TwoFactorAuth/TwoFactorAuthentication.php
@@ -52,7 +52,7 @@ class TwoFactorAuthentication
         $this->secretGenerator = $twoFaSecretRandomGenerator;
     }
 
-    private function getUserModel()
+    private static function getUserModel()
     {
         return new Model();
     }
@@ -70,14 +70,14 @@ class TwoFactorAuthentication
         Piwik::postEvent('TwoFactorAuth.disabled', array($login));
     }
 
-    private function isAnonymous($login)
+    private static function isAnonymous($login)
     {
         return strtolower($login) === 'anonymous';
     }
 
     public function saveSecret($login, $secret)
     {
-        if ($this->isAnonymous($login)) {
+        if (self::isAnonymous($login)) {
             throw new Exception('Anonymous cannot use two-factor authentication');
         }
 
@@ -86,7 +86,7 @@ class TwoFactorAuthentication
             throw new Exception('Cannot enable two-factor authentication, no recovery codes have been created');
         }
 
-        $model = $this->getUserModel();
+        $model = self::getUserModel();
         $model->updateUserFields($login, array('twofactor_secret' => $secret));
     }
 
@@ -95,19 +95,19 @@ class TwoFactorAuthentication
         return $this->settings->twoFactorAuthRequired->getValue();
     }
 
-    public function isUserUsingTwoFactorAuthentication($login)
+    public static function isUserUsingTwoFactorAuthentication($login)
     {
-        if ($this->isAnonymous($login)) {
+        if (self::isAnonymous($login)) {
             return false; // not possible to use auth code with anonymous
         }
 
-        $user = $this->getUser($login);
+        $user = self::getUser($login);
         return !empty($user['twofactor_secret']);
     }
 
-    private function getUser($login)
+    private static function getUser($login)
     {
-        $model = $this->getUserModel();
+        $model = self::getUserModel();
         return $model->getUser($login);
     }
 
@@ -158,11 +158,11 @@ class TwoFactorAuthentication
 
     public function validateAuthCode($login, $authCode)
     {
-        if (!$this->isUserUsingTwoFactorAuthentication($login)) {
+        if (!self::isUserUsingTwoFactorAuthentication($login)) {
             return false;
         }
 
-        $user = $this->getUser($login);
+        $user = self::getUser($login);
 
         if ($this->wasTwoFaCodeUsedRecently($user['login'], $authCode)) {
             return false;
diff --git a/plugins/TwoFactorAuth/Validator.php b/plugins/TwoFactorAuth/Validator.php
index 382ad5f99c..f33387d354 100644
--- a/plugins/TwoFactorAuth/Validator.php
+++ b/plugins/TwoFactorAuth/Validator.php
@@ -59,14 +59,14 @@ class Validator
 
     public function check2FaEnabled()
     {
-        if (!$this->twoFa->isUserUsingTwoFactorAuthentication(Piwik::getCurrentUserLogin())) {
+        if (!TwoFactorAuthentication::isUserUsingTwoFactorAuthentication(Piwik::getCurrentUserLogin())) {
             throw new Exception('not available');
         }
     }
 
     public function check2FaNotEnabled()
     {
-        if ($this->twoFa->isUserUsingTwoFactorAuthentication(Piwik::getCurrentUserLogin())) {
+        if (TwoFactorAuthentication::isUserUsingTwoFactorAuthentication(Piwik::getCurrentUserLogin())) {
             throw new Exception('not available');
         }
     }
diff --git a/plugins/TwoFactorAuth/lang/en.json b/plugins/TwoFactorAuth/lang/en.json
index 77927705f6..b2757a4778 100644
--- a/plugins/TwoFactorAuth/lang/en.json
+++ b/plugins/TwoFactorAuth/lang/en.json
@@ -44,6 +44,8 @@
     "RecoveryCodesAllUsed": "All recovery codes have been used, it is highly recommended you regenerate your recovery codes.",
     "RecoveryCodesRegenerated": "Recovery codes have been regenerated. Make sure to download or print the newly generated codes.",
     "GenerateNewRecoveryCodes": "Generate new recovery codes",
-    "GenerateNewRecoveryCodesInfo": "When you generate new recovery codes, your old codes won’t work anymore. Make sure to download or print your new codes."
+    "GenerateNewRecoveryCodesInfo": "When you generate new recovery codes, your old codes won’t work anymore. Make sure to download or print your new codes.",
+    "RequireTwoFAForAll": "Require two-factor authentication for everyone",
+    "RequireTwoFAForAllInformation": "When enabled, every user has to enable two factor authentication. Enforcing 2FA will require all users to have access to a device where they can install an authenticator app. You can only enable this if you already have two factor authentication set up yourself."
   }
 }
\ No newline at end of file
diff --git a/plugins/TwoFactorAuth/tests/Integration/APITest.php b/plugins/TwoFactorAuth/tests/Integration/APITest.php
index 500076d246..af73c410d3 100644
--- a/plugins/TwoFactorAuth/tests/Integration/APITest.php
+++ b/plugins/TwoFactorAuth/tests/Integration/APITest.php
@@ -72,11 +72,11 @@ class APITest extends IntegrationTestCase
         $this->twoFa->saveSecret('mylogin1', '1234');
         $this->twoFa->saveSecret('mylogin2', '1234');
 
-        $this->assertTrue($this->twoFa->isUserUsingTwoFactorAuthentication('mylogin1'));
-        $this->assertTrue($this->twoFa->isUserUsingTwoFactorAuthentication('mylogin2'));
+        $this->assertTrue(TwoFactorAuthentication::isUserUsingTwoFactorAuthentication('mylogin1'));
+        $this->assertTrue(TwoFactorAuthentication::isUserUsingTwoFactorAuthentication('mylogin2'));
         $this->api->resetTwoFactorAuth('mylogin1');
-        $this->assertFalse($this->twoFa->isUserUsingTwoFactorAuthentication('mylogin1'));
-        $this->assertTrue($this->twoFa->isUserUsingTwoFactorAuthentication('mylogin2'));
+        $this->assertFalse(TwoFactorAuthentication::isUserUsingTwoFactorAuthentication('mylogin1'));
+        $this->assertTrue(TwoFactorAuthentication::isUserUsingTwoFactorAuthentication('mylogin2'));
 
         $this->assertEquals([], $this->recoveryCodes->getAllRecoveryCodesForLogin('mylogin1'));
     }
diff --git a/plugins/TwoFactorAuth/tests/Integration/TwoFactorAuthenticationTest.php b/plugins/TwoFactorAuth/tests/Integration/TwoFactorAuthenticationTest.php
index 156e2ca9ef..3b466e6a04 100644
--- a/plugins/TwoFactorAuth/tests/Integration/TwoFactorAuthenticationTest.php
+++ b/plugins/TwoFactorAuth/tests/Integration/TwoFactorAuthenticationTest.php
@@ -74,15 +74,15 @@ class TwoFactorAuthenticationTest extends IntegrationTestCase
     {
         $this->dao->createRecoveryCodesForLogin('mylogin');
 
-        $this->assertFalse($this->twoFa->isUserUsingTwoFactorAuthentication('mylogin'));
+        $this->assertFalse(TwoFactorAuthentication::isUserUsingTwoFactorAuthentication('mylogin'));
         $this->twoFa->saveSecret('mylogin', '123456');
 
-        $this->assertTrue($this->twoFa->isUserUsingTwoFactorAuthentication('mylogin'));
-        $this->assertFalse($this->twoFa->isUserUsingTwoFactorAuthentication('mylogin2'));
+        $this->assertTrue(TwoFactorAuthentication::isUserUsingTwoFactorAuthentication('mylogin'));
+        $this->assertFalse(TwoFactorAuthentication::isUserUsingTwoFactorAuthentication('mylogin2'));
 
         $this->twoFa->disable2FAforUser('mylogin');
 
-        $this->assertFalse($this->twoFa->isUserUsingTwoFactorAuthentication('mylogin'));
+        $this->assertFalse(TwoFactorAuthentication::isUserUsingTwoFactorAuthentication('mylogin'));
     }
 
     public function test_disable2FAforUser_removesAllRecoveryCodes()
@@ -111,7 +111,7 @@ class TwoFactorAuthenticationTest extends IntegrationTestCase
 
     public function test_isUserUsingTwoFactorAuthentication_neverWorksForAnonymous()
     {
-        $this->assertFalse($this->twoFa->isUserUsingTwoFactorAuthentication('anonymous'));
+        $this->assertFalse(TwoFactorAuthentication::isUserUsingTwoFactorAuthentication('anonymous'));
     }
 
     public function test_validateAuthCodeDuringSetup()