diff --git a/CHANGELOG.md b/CHANGELOG.md index e8ec352..a210f0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # CHANGELOG ## work in progress + - Enh: Added minimum requirements when a new password is automatically generated (MatteoF96) - Fix #380: Avoid rewriting AccessRule::matchRole (maxxer) - Fix #378: Add module attribute 'disableIpLogging' (jkmssoft) - Enh #387: Added Persian translation (hadi-aj) diff --git a/docs/installation/configuration-options.md b/docs/installation/configuration-options.md index 5cc51f8..e95f165 100644 --- a/docs/installation/configuration-options.md +++ b/docs/installation/configuration-options.md @@ -220,5 +220,17 @@ Set to `true` to restrict user assignments to roles only. If `true` registration and last login IPs are not logged into users table, instead a dummy 127.0.0.1 is used +#### minPasswordRequirements (type: `array`, default: `['lower' => 1, 'digit' => 1, 'upper' => 1]`) + +Minimum requirements when a new password is automatically generated. +Array structure: `"requirement" => minimum_number_characters`. + +Possible array keys: +- lower: minimum number of lowercase characters; +- upper: minimum number of uppercase characters; +- digit: minimum number of digits; +- special: minimum number of special characters; +- min: minimum number of characters (= minimum length). + © [2amigos](http://www.2amigos.us/) 2013-2019 diff --git a/src/User/Helper/SecurityHelper.php b/src/User/Helper/SecurityHelper.php index f540f0d..5fec4ab 100644 --- a/src/User/Helper/SecurityHelper.php +++ b/src/User/Helper/SecurityHelper.php @@ -11,8 +11,10 @@ namespace Da\User\Helper; +use Yii; use yii\base\Exception; use yii\base\Security; +use yii\base\InvalidConfigException; class SecurityHelper { @@ -60,25 +62,48 @@ class SecurityHelper return $this->security->validatePassword($password, $hash); } - public function generatePassword($length) + public function generatePassword($length, $minPasswordRequirements = null) { $sets = [ - 'abcdefghjkmnpqrstuvwxyz', - 'ABCDEFGHJKMNPQRSTUVWXYZ', - '23456789', + 'lower' => 'abcdefghjkmnpqrstuvwxyz', + 'upper' => 'ABCDEFGHJKMNPQRSTUVWXYZ', + 'digit' => '123456789', + 'special' => '!#$%&*+,-.:;<=>?@_~' ]; $all = ''; $password = ''; - foreach ($sets as $set) { - $password .= $set[array_rand(str_split($set))]; + + if (!isset($minPasswordRequirements)) { + if (isset(Yii::$app->getModule('user')->minPasswordRequirements)) { + $minPasswordRequirements = Yii::$app->getModule('user')->minPasswordRequirements; + } + else { + $minPasswordRequirements = [ + 'lower' => 1, + 'digit' => 1, + 'upper' => 1, + ]; + } + } + if (isset($minPasswordRequirements['min']) && $length < $minPasswordRequirements['min']) { + $length = $minPasswordRequirements['min']; + } + foreach ($sets as $setKey => $set) { + if (isset($minPasswordRequirements[$setKey])) { + for ($i = 0; $i < $minPasswordRequirements[$setKey]; $i++) { + $password .= $set[array_rand(str_split($set))]; + } + } $all .= $set; } - + $passwordLength = strlen($password); + if ($passwordLength > $length) { + throw new InvalidConfigException('The minimum length is incompatible with other minimum requirements.'); + } $all = str_split($all); - for ($i = 0; $i < $length - count($sets); ++$i) { + for ($i = 0; $i < $length - $passwordLength; ++$i) { $password .= $all[array_rand($all)]; } - $password = str_shuffle($password); return $password; diff --git a/src/User/Module.php b/src/User/Module.php index 26b8b17..f86c8da 100644 --- a/src/User/Module.php +++ b/src/User/Module.php @@ -210,6 +210,23 @@ class Module extends BaseModule * @var boolean whether to disable IP logging into user table */ public $disableIpLogging = false; + + /** + * @var array Minimum requirements when a new password is automatically generated. + * Array structure: `requirement => minimum number characters`. + * + * Possible array keys: + * - lower: minimum number of lowercase characters; + * - upper: minimum number of uppercase characters; + * - digit: minimum number of digits; + * - special: minimum number of special characters; + * - min: minimum number of characters (= minimum length). + */ + public $minPasswordRequirements = [ + 'lower' => 1, + 'digit' => 1, + 'upper' => 1, + ]; /** * @return string with the hit to be used with the give consent checkbox diff --git a/src/User/Service/UserCreateService.php b/src/User/Service/UserCreateService.php index 60ff7eb..5f7622d 100644 --- a/src/User/Service/UserCreateService.php +++ b/src/User/Service/UserCreateService.php @@ -57,7 +57,7 @@ class UserCreateService implements ServiceInterface $model->confirmed_at = time(); $model->password = !empty($model->password) ? $model->password - : $this->securityHelper->generatePassword(8); + : $this->securityHelper->generatePassword(8, $this->getModule('user')->minPasswordRequirements); /** @var UserEvent $event */ $event = $this->make(UserEvent::class, [$model]); diff --git a/src/User/Service/UserRegisterService.php b/src/User/Service/UserRegisterService.php index eb15983..747d9eb 100644 --- a/src/User/Service/UserRegisterService.php +++ b/src/User/Service/UserRegisterService.php @@ -51,7 +51,7 @@ class UserRegisterService implements ServiceInterface try { $model->confirmed_at = $this->getModule()->enableEmailConfirmation ? null : time(); $model->password = $this->getModule()->generatePasswords - ? $this->securityHelper->generatePassword(8) + ? $this->securityHelper->generatePassword(8, $this->getModule('user')->minPasswordRequirements) : $model->password; $event = $this->make(UserEvent::class, [$model]); diff --git a/tests/unit/GeneratePasswordTest.php b/tests/unit/GeneratePasswordTest.php new file mode 100644 index 0000000..2e4168f --- /dev/null +++ b/tests/unit/GeneratePasswordTest.php @@ -0,0 +1,160 @@ + 'abcdefghjkmnpqrstuvwxyz', + * 'upper' => 'ABCDEFGHJKMNPQRSTUVWXYZ', + * 'digit' => '123456789', + * 'special' => '!#$%&*+,-.:;<=>?@_~' + * ]; + */ +class GeneratePasswordTest extends \Codeception\Test\Unit +{ + const ITERATIONS = 10000; + + // Test with minPasswordRequirements equal to null (get default value/parameter) + public function testNullParameter () + { + $length = 8; + $minPasswordRequirements = null; + // Helper + $securityHelper = new SecurityHelper(new Security()); // Empty security (it does not matter) + // Check password correctness + $ok = true; + for ($i = 0; $i < self::ITERATIONS; $i++) { + $password = $securityHelper->generatePassword($length, $minPasswordRequirements); + $result = preg_match('/\A(?=(.*\d){1})(?=(?:[^a-z]*[a-z]){1})(?=(?:[^A-Z]*[A-Z]){1})[0-9a-zA-Z!#$%&*+,-.:;<=>?@_~]{8,}\z/', $password); + if ($result === 0) { + $ok = false; + break; + } + } + $this->assertTrue($ok); + } + + // Test with minPasswordRequirements equal to an empty array (= password without requirements) + public function testEmptyParameter () + { + $length = 8; + $minPasswordRequirements = []; + // Helper + $securityHelper = new SecurityHelper(new Security()); // Empty security (it does not matter) + // Check password correctness + $ok = true; + for ($i = 0; $i < self::ITERATIONS; $i++) { + $password = $securityHelper->generatePassword($length, $minPasswordRequirements); + $result = preg_match('/\A[0-9a-zA-Z!#$%&*+,-.:;<=>?@_~]{8,}\z/', $password); + if ($result === 0) { + $ok = false; + break; + } + } + $this->assertTrue($ok); + } + + // Test with many lowercase characters, one uppercase character, one digit and one special character + public function testManyLowercaseCharacter () + { + // Function parameters + $length = 8; + $minPasswordRequirements = [ + 'min' => 10, + 'special' => 1, + 'digit' => 1, + 'upper' => 1, + 'lower' => 5 + ]; + // Helper + $securityHelper = new SecurityHelper(new Security()); // Empty security (it does not matter) + // Check password correctness + $ok = true; + for ($i = 0; $i < self::ITERATIONS; $i++) { + $password = $securityHelper->generatePassword($length, $minPasswordRequirements); + $result = preg_match('/\A(?=(.*\d){1})(?=(?:[^a-z]*[a-z]){5})(?=(?:[^A-Z]*[A-Z]){1})(?=(?:[0-9a-zA-Z]*[!#$%&*+,-.:;<=>?@_~]){1})[0-9a-zA-Z!#$%&*+,-.:;<=>?@_~]{10,}\z/', $password); + if ($result === 0) { + $ok = false; + break; + } + } + $this->assertTrue($ok); + } + + // Test with many special characters, one uppercase character, one digit + public function testManySpecialCharacter () + { + // Function parameters + $length = 10; + $minPasswordRequirements = [ + 'min' => 10, + 'special' => 6, + 'digit' => 1, + 'upper' => 1, + ]; + // Helper + $securityHelper = new SecurityHelper(new Security()); // Empty security (it does not matter) + // Check password correctness + $ok = true; + for ($i = 0; $i < self::ITERATIONS; $i++) { + $password = $securityHelper->generatePassword($length, $minPasswordRequirements); + $result = preg_match('/\A(?=(.*\d){1})(?=(?:[^A-Z]*[A-Z]){1})(?=(?:[0-9a-zA-Z]*[!#$%&*+,-.:;<=>?@_~]){6})[0-9a-zA-Z!#$%&*+,-.:;<=>?@_~]{10,}\z/', $password); + if ($result === 0) { + $ok = false; + break; + } + } + $this->assertTrue($ok); + } + + // Test with a long password and no requirements + public function testLongPassword () + { + // Function parameters + $length = 20; + $minPasswordRequirements = []; + // Helper + $securityHelper = new SecurityHelper(new Security()); // Empty security (it does not matter) + // Check password correctness + $ok = true; + for ($i = 0; $i < self::ITERATIONS; $i++) { + $password = $securityHelper->generatePassword($length, $minPasswordRequirements); + $result = preg_match('/\A[0-9a-zA-Z!#$%&*+,-.:;<=>?@_~]{20,}\z/', $password); + if ($result === 0) { + $ok = false; + break; + } + } + $this->assertTrue($ok); + } + + // Test with random requirements + public function testRandomRequirements () + { + // Function parameters + $length = 8; + $minPasswordRequirements = [ + 'min' => 10, + 'special' => 4, + 'digit' => 3, + 'upper' => 2, + 'lower' => 1 + ]; + // Helper + $securityHelper = new SecurityHelper(new Security()); // Empty security (it does not matter) + // Check password correctness + $ok = true; + for ($i = 0; $i < self::ITERATIONS; $i++) { + $password = $securityHelper->generatePassword($length, $minPasswordRequirements); + $result = preg_match('/\A(?=(.*\d){3})(?=(?:[^a-z]*[a-z]){1})(?=(?:[^A-Z]*[A-Z]){2})(?=(?:[0-9a-zA-Z]*[!#$%&*+,-.:;<=>?@_~]){4})[0-9a-zA-Z!#$%&*+,-.:;<=>?@_~]{10,}\z/', $password); + if ($result === 0) { + $ok = false; + break; + } + } + $this->assertTrue($ok); + } +} \ No newline at end of file