diff options
author | Andrew Davis <andrew.affinity@gmail.com> | 2021-04-14 01:17:51 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-04-14 01:17:51 +0300 |
commit | beb2699903a8bf92411dcf11945161d518ad6044 (patch) | |
tree | 2c4d1dca01168fe8fad769106240d8345e30bf68 /plugins | |
parent | 4ac51607847c83e93fb2752484458b0040e4ed0c (diff) |
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
Diffstat (limited to 'plugins')
-rw-r--r-- | plugins/Tour/Engagement/ChallengeSetupTwoFa.php | 4 | ||||
-rw-r--r-- | plugins/TwoFactorAuth/Controller.php | 6 | ||||
-rw-r--r-- | plugins/TwoFactorAuth/SystemSettings.php | 16 | ||||
-rw-r--r-- | plugins/TwoFactorAuth/TwoFactorAuth.php | 10 | ||||
-rw-r--r-- | plugins/TwoFactorAuth/TwoFactorAuthentication.php | 22 | ||||
-rw-r--r-- | plugins/TwoFactorAuth/Validator.php | 4 | ||||
-rw-r--r-- | plugins/TwoFactorAuth/lang/en.json | 4 | ||||
-rw-r--r-- | plugins/TwoFactorAuth/tests/Integration/APITest.php | 8 | ||||
-rw-r--r-- | plugins/TwoFactorAuth/tests/Integration/TwoFactorAuthenticationTest.php | 10 |
9 files changed, 47 insertions, 37 deletions
diff --git a/plugins/Tour/Engagement/ChallengeSetupTwoFa.php b/plugins/Tour/Engagement/ChallengeSetupTwoFa.php index dc5fa1c65c..9542fc8633 100644 --- a/plugins/Tour/Engagement/ChallengeSetupTwoFa.php +++ b/plugins/Tour/Engagement/ChallengeSetupTwoFa.php @@ -8,7 +8,6 @@ */ namespace Piwik\Plugins\Tour\Engagement; -use Piwik\Container\StaticContainer; use Piwik\Piwik; use Piwik\Plugins\TwoFactorAuth\TwoFactorAuthentication; @@ -31,8 +30,7 @@ class ChallengeSetupTwoFa extends Challenge public function isCompleted() { - $twoFa = StaticContainer::get(TwoFactorAuthentication::class); - return $twoFa->isUserUsingTwoFactorAuthentication(Piwik::getCurrentUserLogin()); + return TwoFactorAuthentication::isUserUsingTwoFactorAuthentication(Piwik::getCurrentUserLogin()); } public function getUrl() 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() |