diff --git a/application/Espo/Core/Formula/Functions/PasswordGroup/GenerateType.php b/application/Espo/Core/Formula/Functions/PasswordGroup/GenerateType.php index 159ed3d862..31d4765699 100644 --- a/application/Espo/Core/Formula/Functions/PasswordGroup/GenerateType.php +++ b/application/Espo/Core/Formula/Functions/PasswordGroup/GenerateType.php @@ -29,30 +29,18 @@ namespace Espo\Core\Formula\Functions\PasswordGroup; -use Espo\Core\Formula\{ - Functions\BaseFunction, - ArgumentList, -}; +use Espo\Core\Formula\EvaluatedArgumentList; +use Espo\Core\Formula\Func; +use Espo\Tools\UserSecurity\Password\Generator; -use Espo\Core\Utils\Util; - -use Espo\Core\Di; - -class GenerateType extends BaseFunction implements - Di\ConfigAware +class GenerateType implements Func { - use Di\ConfigSetter; + public function __construct( + private Generator $generator, + ) {} - public function process(ArgumentList $args) + public function process(EvaluatedArgumentList $arguments): string { - $config = $this->config; - - $length = $config->get('passwordGenerateLength', 10); - $letterCount = $config->get('passwordGenerateLetterCount', 4); - $numberCount = $config->get('passwordGenerateNumberCount', 2); - - $password = Util::generatePassword($length, $letterCount, $numberCount, true); - - return $password; + return $this->generator->generate(); } } diff --git a/application/Espo/Core/Utils/Util.php b/application/Espo/Core/Utils/Util.php index 37805744e2..176c9c137c 100644 --- a/application/Espo/Core/Utils/Util.php +++ b/application/Espo/Core/Utils/Util.php @@ -983,13 +983,13 @@ class Util * @param int $letters A number of letters. * @param int $digits A number of digits. * @param bool $bothCases Use upper and lower case letters. - * @return string */ public static function generatePassword( int $length = 8, int $letters = 5, int $digits = 3, - bool $bothCases = false + bool $bothCases = false, + int $specialCharacters = 0 ): string { $chars = [ @@ -998,6 +998,7 @@ class Util 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789', 'ABCDEFGHIJKLMNOPQRSTUVWXYZ', 'abcdefghijklmnopqrstuvwxyz', + "'-!\"#$%&()*,./:;?@[]^_`{|}~+<=>", ]; $shuffle = function ($array) { @@ -1030,7 +1031,7 @@ class Util } } - $either = $length - ($letters + $digits + $upperCase + $lowerCase); + $either = $length - ($letters + $digits + $upperCase + $lowerCase + $specialCharacters); if ($either < 0) { $either = 0; @@ -1038,7 +1039,7 @@ class Util $array = []; - foreach ([$letters, $digits, $either, $upperCase, $lowerCase] as $i => $len) { + foreach ([$letters, $digits, $either, $upperCase, $lowerCase, $specialCharacters] as $i => $len) { $set = $chars[$i]; $subArray = []; diff --git a/application/Espo/EntryPoints/ChangePassword.php b/application/Espo/EntryPoints/ChangePassword.php index d8ce7bf8c1..d4362f1ab0 100644 --- a/application/Espo/EntryPoints/ChangePassword.php +++ b/application/Espo/EntryPoints/ChangePassword.php @@ -34,19 +34,19 @@ use Espo\Entities\PasswordChangeRequest; use Espo\Core\Utils\Client\ActionRenderer; use Espo\Core\EntryPoint\EntryPoint; use Espo\Core\EntryPoint\Traits\NoAuth; -use Espo\Core\Utils\Config; use Espo\Core\Api\Request; use Espo\Core\Api\Response; use Espo\ORM\EntityManager; +use Espo\Tools\UserSecurity\Password\ConfigProvider; class ChangePassword implements EntryPoint { use NoAuth; public function __construct( - private Config $config, private EntityManager $entityManager, - private ActionRenderer $actionRenderer + private ActionRenderer $actionRenderer, + private ConfigProvider $configProvider, ) {} public function run(Request $request, Response $response): void @@ -65,13 +65,14 @@ class ChangePassword implements EntryPoint ->findOne(); $strengthParams = [ - 'passwordGenerateLength' => $this->config->get('passwordGenerateLength'), - 'passwordGenerateLetterCount' => $this->config->get('passwordGenerateLetterCount'), - 'generateNumberCount' => $this->config->get('generateNumberCount'), - 'passwordStrengthLength' => $this->config->get('passwordStrengthLength'), - 'passwordStrengthLetterCount' => $this->config->get('passwordStrengthLetterCount'), - 'passwordStrengthNumberCount' => $this->config->get('passwordStrengthNumberCount'), - 'passwordStrengthBothCases' => $this->config->get('passwordStrengthBothCases'), + 'passwordGenerateLength' => $this->configProvider->getGenerateLength(), + 'passwordGenerateLetterCount' => $this->configProvider->getGenerateLetterCount(), + 'generateNumberCount' => $this->configProvider->getGenerateNumberCount(), + 'passwordStrengthLength' => $this->configProvider->getStrengthLength(), + 'passwordStrengthLetterCount' => $this->configProvider->getStrengthLetterCount(), + 'passwordStrengthNumberCount' => $this->configProvider->getStrengthNumberCount(), + 'passwordStrengthBothCases' => $this->configProvider->getStrengthBothCases(), + 'passwordStrengthSpecialCharacterCount' => $this->configProvider->getStrengthSpecialCharacterCount(), ]; $options = [ diff --git a/application/Espo/Resources/defaults/config.php b/application/Espo/Resources/defaults/config.php index 08337c6223..a5302cca3f 100644 --- a/application/Espo/Resources/defaults/config.php +++ b/application/Espo/Resources/defaults/config.php @@ -280,6 +280,11 @@ return [ 'ldapUserObjectClass' => 'person', 'ldapPortalUserLdapAuth' => false, 'passwordGenerateLength' => 10, + 'passwordStrengthLength' => null, + 'passwordStrengthLetterCount' => null, + 'passwordStrengthNumberCount' => null, + 'passwordStrengthBothCases' => false, + 'passwordStrengthSpecialCharacterCount' => null, 'massActionIdleCountThreshold' => 100, 'exportIdleCountThreshold' => 1000, 'clientSecurityHeadersDisabled' => false, diff --git a/application/Espo/Resources/i18n/en_US/Settings.json b/application/Espo/Resources/i18n/en_US/Settings.json index dc7e275e5a..4201dc737d 100644 --- a/application/Espo/Resources/i18n/en_US/Settings.json +++ b/application/Espo/Resources/i18n/en_US/Settings.json @@ -146,6 +146,7 @@ "passwordStrengthLetterCount": "Number of letters required in password", "passwordStrengthNumberCount": "Number of digits required in password", "passwordStrengthBothCases": "Password must contain letters of both upper and lower case", + "passwordStrengthSpecialCharacterCount": "Number of special character required in password", "auth2FA": "Enable 2-Factor Authentication", "auth2FAForced": "Force regular users to set up 2FA", "auth2FAMethodList": "Available 2FA methods", diff --git a/application/Espo/Resources/i18n/en_US/User.json b/application/Espo/Resources/i18n/en_US/User.json index 91e9bbbeb0..e455f0a1be 100644 --- a/application/Espo/Resources/i18n/en_US/User.json +++ b/application/Espo/Resources/i18n/en_US/User.json @@ -107,6 +107,7 @@ "passwordStrengthLength": "Must be at least {length} characters long.", "passwordStrengthLetterCount": "Must contain at least {count} letter(s).", "passwordStrengthNumberCount": "Must contain at least {count} digit(s).", + "passwordStrengthSpecialCharacterCount": "Must contain at least {count} special character(s).", "passwordStrengthBothCases": "Must contain letters of both upper and lower case.", "passwordWillBeSent": "Password will be sent to user's email address.", "passwordChanged": "Password has been changed", diff --git a/application/Espo/Resources/layouts/Settings/authentication.json b/application/Espo/Resources/layouts/Settings/authentication.json index e9652bac53..30a9093c52 100644 --- a/application/Espo/Resources/layouts/Settings/authentication.json +++ b/application/Espo/Resources/layouts/Settings/authentication.json @@ -26,9 +26,11 @@ "tabLabel": "$label:Passwords", "label": "Strength", "rows": [ - [{"name": "passwordGenerateLength"}, false], - [{"name": "passwordStrengthLength"}, {"name": "passwordStrengthLetterCount"}], - [{"name": "passwordStrengthBothCases"}, {"name": "passwordStrengthNumberCount"}] + [{"name": "passwordGenerateLength"}, {"name": "passwordStrengthLength"}], + [false, {"name": "passwordStrengthLetterCount"}], + [false, {"name": "passwordStrengthBothCases"}], + [false, {"name": "passwordStrengthNumberCount"}], + [false, {"name": "passwordStrengthSpecialCharacterCount"}] ] }, { diff --git a/application/Espo/Resources/metadata/entityDefs/Settings.json b/application/Espo/Resources/metadata/entityDefs/Settings.json index d651c623bf..69fb0cb9f9 100644 --- a/application/Espo/Resources/metadata/entityDefs/Settings.json +++ b/application/Espo/Resources/metadata/entityDefs/Settings.json @@ -332,6 +332,11 @@ "max": 150, "min": 0 }, + "passwordStrengthSpecialCharacterCount": { + "type": "int", + "max": 50, + "min": 0 + }, "passwordStrengthBothCases": { "type": "bool" }, diff --git a/application/Espo/Tools/UserSecurity/Password/Checker.php b/application/Espo/Tools/UserSecurity/Password/Checker.php index 0db76529ea..909e68a275 100644 --- a/application/Espo/Tools/UserSecurity/Password/Checker.php +++ b/application/Espo/Tools/UserSecurity/Password/Checker.php @@ -29,21 +29,17 @@ namespace Espo\Tools\UserSecurity\Password; -use Espo\Core\Utils\Config; - class Checker { - private Config $config; + private const SPECIAL_CHARACTERS = "'-!\"#$%&()*,./:;?@[]^_`{|}~+<=>"; public function __construct( - Config $config - ) { - $this->config = $config; - } + private ConfigProvider $configProvider, + ) {} public function checkStrength(string $password): bool { - $minLength = $this->config->get('passwordStrengthLength'); + $minLength = $this->configProvider->getStrengthLength(); if ($minLength) { if (mb_strlen($password) < $minLength) { @@ -51,7 +47,7 @@ class Checker } } - $requiredLetterCount = $this->config->get('passwordStrengthLetterCount'); + $requiredLetterCount = $this->configProvider->getStrengthLetterCount(); if ($requiredLetterCount) { $letterCount = 0; @@ -67,7 +63,7 @@ class Checker } } - $requiredNumberCount = $this->config->get('passwordStrengthNumberCount'); + $requiredNumberCount = $this->configProvider->getStrengthNumberCount(); if ($requiredNumberCount) { $numberCount = 0; @@ -83,7 +79,7 @@ class Checker } } - $bothCases = $this->config->get('passwordStrengthBothCases'); + $bothCases = $this->configProvider->getStrengthBothCases(); if ($bothCases) { $ucCount = 0; @@ -103,6 +99,22 @@ class Checker } } + $specialCharacterCount = $this->configProvider->getStrengthSpecialCharacterCount(); + + if ($specialCharacterCount) { + $realSpecialCharacterCount = 0; + + foreach (str_split($password) as $c) { + if (str_contains(self::SPECIAL_CHARACTERS, $c)) { + $realSpecialCharacterCount++; + } + } + + if ($realSpecialCharacterCount < $specialCharacterCount) { + return false; + } + } + return true; } } diff --git a/application/Espo/Tools/UserSecurity/Password/ConfigProvider.php b/application/Espo/Tools/UserSecurity/Password/ConfigProvider.php new file mode 100644 index 0000000000..a76f2eb700 --- /dev/null +++ b/application/Espo/Tools/UserSecurity/Password/ConfigProvider.php @@ -0,0 +1,79 @@ +. + * + * The interactive user interfaces in modified source and object code versions + * of this program must display Appropriate Legal Notices, as required under + * Section 5 of the GNU Affero General Public License version 3. + * + * In accordance with Section 7(b) of the GNU Affero General Public License version 3, + * these Appropriate Legal Notices must retain the display of the "EspoCRM" word. + ************************************************************************/ + +namespace Espo\Tools\UserSecurity\Password; + +use Espo\Core\Utils\Config; + +class ConfigProvider +{ + + public function __construct( + private Config $config, + ) {} + + public function getStrengthLength(): ?int + { + return $this->config->get('passwordStrengthLength'); + } + + public function getStrengthLetterCount(): ?int + { + return $this->config->get('passwordStrengthLetterCount'); + } + + public function getStrengthNumberCount(): ?int + { + return $this->config->get('passwordStrengthNumberCount'); + } + + public function getStrengthSpecialCharacterCount(): ?int + { + return $this->config->get('passwordStrengthSpecialCharacterCount'); + } + + public function getStrengthBothCases(): bool + { + return (bool) $this->config->get('passwordStrengthBothCases'); + } + + public function getGenerateLength(): ?int + { + return $this->config->get('passwordGenerateLength'); + } + public function getGenerateLetterCount(): ?int + { + return $this->config->get('passwordGenerateLetterCount'); + } + + public function getGenerateNumberCount(): ?int + { + return $this->config->get('passwordGenerateNumberCount'); + } +} diff --git a/application/Espo/Tools/UserSecurity/Password/Generator.php b/application/Espo/Tools/UserSecurity/Password/Generator.php index 0e7be82114..89ba91edc6 100644 --- a/application/Espo/Tools/UserSecurity/Password/Generator.php +++ b/application/Espo/Tools/UserSecurity/Password/Generator.php @@ -29,7 +29,6 @@ namespace Espo\Tools\UserSecurity\Password; -use Espo\Core\Utils\Config; use Espo\Core\Utils\Util; /** @@ -39,30 +38,27 @@ use Espo\Core\Utils\Util; */ class Generator { - private Config $config; - public function __construct( - Config $config - ) { - $this->config = $config; - } + private ConfigProvider $configProvider, + ) {} /** * Generate a password. */ public function generate(): string { - $length = $this->config->get('passwordStrengthLength'); - $letterCount = $this->config->get('passwordStrengthLetterCount'); - $numberCount = $this->config->get('passwordStrengthNumberCount'); + $length = $this->configProvider->getStrengthLength(); + $letterCount = $this->configProvider->getStrengthLetterCount(); + $numberCount = $this->configProvider->getStrengthNumberCount(); + $specialCharacterCount = $this->configProvider->getStrengthSpecialCharacterCount() ?? 0; - $generateLength = $this->config->get('passwordGenerateLength', 10); - $generateLetterCount = $this->config->get('passwordGenerateLetterCount', 4); - $generateNumberCount = $this->config->get('passwordGenerateNumberCount', 2); + $generateLength = $this->configProvider->getGenerateLength() ?? 10; + $generateLetterCount = $this->configProvider->getGenerateLetterCount() ?? 4; + $generateNumberCount = $this->configProvider->getGenerateNumberCount() ?? 2; $length = is_null($length) ? $generateLength : $length; $letterCount = is_null($letterCount) ? $generateLetterCount : $letterCount; - $numberCount = is_null($letterCount) ? $generateNumberCount : $numberCount; + $numberCount = is_null($numberCount) ? $generateNumberCount : $numberCount; if ($length < $generateLength) { $length = $generateLength; @@ -76,6 +72,6 @@ class Generator $numberCount = $generateNumberCount; } - return Util::generatePassword($length, $letterCount, $numberCount, true); + return Util::generatePassword($length, $letterCount, $numberCount, true, $specialCharacterCount); } } diff --git a/client/src/views/user/fields/generate-password.js b/client/src/views/user/fields/generate-password.js index 13ef5a05ea..c2a4ad7fe1 100644 --- a/client/src/views/user/fields/generate-password.js +++ b/client/src/views/user/fields/generate-password.js @@ -72,6 +72,9 @@ class UserGeneratePasswordFieldView extends BaseFieldView { this.passwordStrengthNumberCount = this.strengthParams.passwordStrengthNumberCount || this.getConfig().get('passwordStrengthNumberCount'); + this.passwordStrengthSpecialCharacterCount = this.strengthParams.passwordStrengthSpecialCharacterCount || + this.getConfig().get('passwordStrengthSpecialCharacterCount'); + this.passwordGenerateLength = this.strengthParams.passwordGenerateLength || this.getConfig().get('passwordGenerateLength'); @@ -90,6 +93,7 @@ class UserGeneratePasswordFieldView extends BaseFieldView { let length = this.passwordStrengthLength; let letterCount = this.passwordStrengthLetterCount; let numberCount = this.passwordStrengthNumberCount; + const specialCharacterCount = this.passwordStrengthSpecialCharacterCount; const generateLength = this.passwordGenerateLength || 10; const generateLetterCount = this.passwordGenerateLetterCount || 4; @@ -111,7 +115,7 @@ class UserGeneratePasswordFieldView extends BaseFieldView { numberCount = generateNumberCount; } - const password = this.generatePassword(length, letterCount, numberCount, true); + const password = this.generatePassword(length, letterCount, numberCount, true, specialCharacterCount); this.model.set({ password: password, @@ -120,13 +124,23 @@ class UserGeneratePasswordFieldView extends BaseFieldView { }, {isGenerated: true}); } - generatePassword(length, letters, numbers, bothCases) { + /** + * @private + * @param {number} length + * @param {number} letters + * @param {number} numbers + * @param {boolean} bothCases + * @param {number} specialCharacters + * @return {string} + */ + generatePassword(length, letters, numbers, bothCases, specialCharacters) { const chars = [ 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz', '0123456789', 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789', 'ABCDEFGHIJKLMNOPQRSTUVWXYZ', 'abcdefghijklmnopqrstuvwxyz', + "'-!\"#$%&()*,./:;?@[]^_`{|}~+<=>", ]; let upperCase = 0; @@ -143,13 +157,13 @@ class UserGeneratePasswordFieldView extends BaseFieldView { } } - let either = length - (letters + numbers + upperCase + lowerCase); + let either = length - (letters + numbers + upperCase + lowerCase + specialCharacters); if (either < 0) { either = 0; } - const setList = [letters, numbers, either, upperCase, lowerCase]; + const setList = [letters, numbers, either, upperCase, lowerCase, specialCharacters]; const shuffle = function (array) { let currentIndex = array.length, temporaryValue, randomIndex; diff --git a/client/src/views/user/fields/password.js b/client/src/views/user/fields/password.js index 20e8bd7f0a..c625a3d69e 100644 --- a/client/src/views/user/fields/password.js +++ b/client/src/views/user/fields/password.js @@ -44,6 +44,7 @@ class UserPasswordFieldView extends PasswordFieldView { passwordStrengthLetterCount: this.getConfig().get('passwordStrengthLetterCount'), passwordStrengthNumberCount: this.getConfig().get('passwordStrengthNumberCount'), passwordStrengthBothCases: this.getConfig().get('passwordStrengthBothCases'), + passwordStrengthSpecialCharacterCount: this.getConfig().get('passwordStrengthSpecialCharacterCount'), }; const minLength = this.strengthParams.passwordStrengthLength; @@ -81,6 +82,13 @@ class UserPasswordFieldView extends PasswordFieldView { ); } + if (this.strengthParams.passwordStrengthSpecialCharacterCount) { + tooltipItemList.push( + '* ' + this.translate('passwordStrengthSpecialCharacterCount', 'messages', 'User') + .replace('{count}', this.strengthParams.passwordStrengthSpecialCharacterCount.toString()) + ); + } + if (tooltipItemList.length) { this.tooltip = true; this.tooltipText = this.translate('Requirements', 'labels', 'User') + ':\n' + tooltipItemList.join('\n'); @@ -178,6 +186,27 @@ class UserPasswordFieldView extends PasswordFieldView { return true; } } + + const requiredSpecialCharacterCount = this.strengthParams.passwordStrengthSpecialCharacterCount; + + if (requiredSpecialCharacterCount) { + let count = 0; + + password.split('').forEach(c => { + if ("'-!\"#$%&()*,./:;?@[]^_`{|}~+<=>".includes(c)) { + count++; + } + }); + + if (count < requiredSpecialCharacterCount) { + const msg = this.translate('passwordStrengthSpecialCharacterCount', 'messages', 'User') + .replace('{count}', requiredSpecialCharacterCount.toString()); + + this.showValidationMessage(msg); + + return true; + } + } } } diff --git a/tests/unit/Espo/Tools/UserSecurity/Password/CheckerTest.php b/tests/unit/Espo/Tools/UserSecurity/Password/CheckerTest.php new file mode 100644 index 0000000000..e91e135d7b --- /dev/null +++ b/tests/unit/Espo/Tools/UserSecurity/Password/CheckerTest.php @@ -0,0 +1,76 @@ +. + * + * The interactive user interfaces in modified source and object code versions + * of this program must display Appropriate Legal Notices, as required under + * Section 5 of the GNU Affero General Public License version 3. + * + * In accordance with Section 7(b) of the GNU Affero General Public License version 3, + * these Appropriate Legal Notices must retain the display of the "EspoCRM" word. + ************************************************************************/ + +namespace tests\unit\Espo\Tools\UserSecurity\Password; + +use Espo\Tools\UserSecurity\Password\Checker; +use Espo\Tools\UserSecurity\Password\ConfigProvider; +use PHPUnit\Framework\TestCase; + +class CheckerTest extends TestCase +{ + public function testCheck(): void + { + $configProvider = $this->createMock(ConfigProvider::class); + + $configProvider + ->expects($this->any()) + ->method('getStrengthLength') + ->willReturn(6); + + $configProvider + ->expects($this->any()) + ->method('getStrengthLetterCount') + ->willReturn(2); + + $configProvider + ->expects($this->any()) + ->method('getStrengthNumberCount') + ->willReturn(1); + + $configProvider + ->expects($this->any()) + ->method('getStrengthBothCases') + ->willReturn(true); + + $configProvider + ->expects($this->any()) + ->method('getStrengthSpecialCharacterCount') + ->willReturn(1); + + $checker = new Checker($configProvider); + + $this->assertTrue($checker->checkStrength("Aa1:aaaaaaaaa")); + $this->assertFalse($checker->checkStrength("Aa1aaaaaaaaa")); + $this->assertFalse($checker->checkStrength("aa1:aaaaaaaaa")); + $this->assertFalse($checker->checkStrength("aaa:aaaaaaaaa")); + $this->assertFalse($checker->checkStrength("11:11111111")); + $this->assertFalse($checker->checkStrength("Aa:1")); + } +} diff --git a/tests/unit/Espo/Tools/UserSecurity/Password/GeneratorTest.php b/tests/unit/Espo/Tools/UserSecurity/Password/GeneratorTest.php new file mode 100644 index 0000000000..d5d51b912d --- /dev/null +++ b/tests/unit/Espo/Tools/UserSecurity/Password/GeneratorTest.php @@ -0,0 +1,78 @@ +. + * + * The interactive user interfaces in modified source and object code versions + * of this program must display Appropriate Legal Notices, as required under + * Section 5 of the GNU Affero General Public License version 3. + * + * In accordance with Section 7(b) of the GNU Affero General Public License version 3, + * these Appropriate Legal Notices must retain the display of the "EspoCRM" word. + ************************************************************************/ + +namespace tests\unit\Espo\Tools\UserSecurity\Password; + +use Espo\Tools\UserSecurity\Password\ConfigProvider; +use Espo\Tools\UserSecurity\Password\Generator; +use PHPUnit\Framework\TestCase; + +class GeneratorTest extends TestCase +{ + public function testGenerate(): void + { + $configProvider = $this->createMock(ConfigProvider::class); + + $configProvider + ->expects($this->any()) + ->method('getGenerateLength') + ->willReturn(8); + + $configProvider + ->expects($this->any()) + ->method('getStrengthLength') + ->willReturn(6); + + $configProvider + ->expects($this->any()) + ->method('getStrengthLetterCount') + ->willReturn(2); + + $configProvider + ->expects($this->any()) + ->method('getStrengthNumberCount') + ->willReturn(1); + + $configProvider + ->expects($this->any()) + ->method('getStrengthBothCases') + ->willReturn(true); + + $configProvider + ->expects($this->any()) + ->method('getStrengthSpecialCharacterCount') + ->willReturn(1); + + $generator = new Generator($configProvider); + + $password = $generator->generate(); + + $this->assertEquals(8, strlen($password)); + } +}