diff --git a/htdocs/admin/security.php b/htdocs/admin/security.php index a78ea3e912d..795a0557f16 100644 --- a/htdocs/admin/security.php +++ b/htdocs/admin/security.php @@ -319,7 +319,7 @@ if ($conf->global->USER_PASSWORD_GENERATED == "Perso") { print ''; print ''.$langs->trans("NoAmbiCaracAutoGeneration").""; - print ' '.($tabConf[5] ? $langs->trans("Activated") : $langs->trans("Disabled")).''; + print ' '; print ''; print ''; diff --git a/htdocs/core/modules/security/generate/README b/htdocs/core/modules/security/generate/README index c47cee09469..23dc7dcdd91 100644 --- a/htdocs/core/modules/security/generate/README +++ b/htdocs/core/modules/security/generate/README @@ -4,8 +4,7 @@ Adding your own password generator module ------------------------------------ -If you want to add your own password generator module. This is steps to follow -to add you own password generator: +If you want to add your own password generator module. This is steps to follow to add your own password generator: ***** STEP 1 ***** @@ -13,18 +12,16 @@ to add you own password generator: Copy file htdocs/core/modules/security/modGeneratePassNone.class.php into -htdocs/core/modules/mailings/modMyGenerator.class.php +htdocs/core/modules/mailings/modGeneratePassMygenerator.class.php -You can choose value of your choice instead of "myGenerator" in name -of new file. +You can choose value of your choice instead of "Mygenerator" in name of new file. ***** STEP 2 ***** -Edit this file modMyGenerator.class.php and change following text: +Edit this file modGeneratePassMygenerator.class.php and change following text: -"class modGeneratePassNone" into "class modMyGenerator" -"function modGeneratePassNone" into "function modMyGenerator" +"class modGeneratePassNone" into "class modGeneratePassMygenerator" Then add code inside the "getDecription" function. Then add code inside the "getExample" function. diff --git a/htdocs/core/modules/security/generate/modGeneratePassNone.class.php b/htdocs/core/modules/security/generate/modGeneratePassNone.class.php index e6b49b6b232..c6cfb733f73 100644 --- a/htdocs/core/modules/security/generate/modGeneratePassNone.class.php +++ b/htdocs/core/modules/security/generate/modGeneratePassNone.class.php @@ -111,10 +111,11 @@ class modGeneratePassNone extends ModeleGenPassword } /** - * Validate a password + * Validate a password. + * This function is called by User->setPassword() and internally to validate that the password matches the constraints. * - * @param string $password Password to check - * @return int 0 if KO, >0 if OK + * @param string $password Password to check + * @return int 0 if KO, >0 if OK */ public function validatePassword($password) { diff --git a/htdocs/core/modules/security/generate/modGeneratePassPerso.class.php b/htdocs/core/modules/security/generate/modGeneratePassPerso.class.php index 0ecf814cb35..baa818aa63b 100644 --- a/htdocs/core/modules/security/generate/modGeneratePassPerso.class.php +++ b/htdocs/core/modules/security/generate/modGeneratePassPerso.class.php @@ -55,7 +55,13 @@ class modGeneratePassPerso extends ModeleGenPassword public $NbNum; public $NbSpe; public $NbRepeat; - public $WithoutAmbi; + + /** + * Flag to 1 if we must clean ambiguous charaters for the autogeneration of password (List of ambiguous char is in $this->Ambi) + * + * @var integer + */ + public $WithoutAmbi = 0; /** * @var DoliDB Database handler. @@ -92,7 +98,7 @@ class modGeneratePassPerso extends ModeleGenPassword $this->user = $user; if (empty($conf->global->USER_PASSWORD_PATTERN)) { - // default value at auto generation (12 chars, 1 upercase, 1 digit, 1 special char, 3 repeat, no ambi). + // default value at auto generation (12 chars, 1 upercase, 1 digit, 1 special char, 3 repeat, exclude ambiguous characters). dolibarr_set_const($db, "USER_PASSWORD_PATTERN", '12;1;1;1;3;1', 'chaine', 0, '', $conf->entity); } @@ -109,7 +115,15 @@ class modGeneratePassPerso extends ModeleGenPassword $this->NbSpe = $tabConf[3]; $this->NbRepeat = $tabConf[4]; $this->WithoutAmbi = $tabConf[5]; + } + /** + * Init the property ->All and clean ->Maj, ->Min, ->Nb and ->Spe with list of valid chars + * + * @return void + */ + private function initAll() + { if ($this->WithoutAmbi) { $this->Maj = str_replace($this->Ambi, "", $this->Maj); $this->Min = str_replace($this->Ambi, "", $this->Min); @@ -119,16 +133,12 @@ class modGeneratePassPerso extends ModeleGenPassword $pattern = $this->Min.(!empty($this->NbMaj) ? $this->Maj : '').(!empty($this->NbNum) ? $this->Nb : '').(!empty($this->NbSpe) ? $this->Spe : ''); $this->All = str_shuffle($pattern); - - //$this->All = str_shuffle($this->Maj. $this->Min. $this->Nb. $this->Spe); - //$this->All = $this->Maj. $this->Min. $this->Nb. $this->Spe; - //$this->All = $this->Spe; } /** - * Return description of module + * Return description of module * - * @return string Description of text + * @return string Description of text */ public function getDescription() { @@ -137,9 +147,9 @@ class modGeneratePassPerso extends ModeleGenPassword } /** - * Return an example of password generated by this module + * Return an example of password generated by this module * - * @return string Example of password + * @return string Example of password */ public function getExample() { @@ -153,6 +163,8 @@ class modGeneratePassPerso extends ModeleGenPassword */ public function getNewGeneratedPassword() { + $this->initAll(); + $pass = ""; for ($i = 0; $i < $this->NbMaj; $i++) { // Y @@ -180,67 +192,95 @@ class modGeneratePassPerso extends ModeleGenPassword return $pass; } - return $this->getNewGeneratedPassword(); + return $this->getNewGeneratedPassword(); // warning, may generate infinite loop if conditions are not possible } /** - * Validate a password + * Validate a password. + * This function is called by User->setPassword() and internally to validate that the password matches the constraints. * * @param string $password Password to check - * @return bool false if KO, true if OK + * @return int 0 if KO, >0 if OK */ public function validatePassword($password) { + global $langs; + + $this->initAll(); // For the case this method is called alone + + $password_a = preg_split('//u', $password, null, PREG_SPLIT_NO_EMPTY); + $maj = preg_split('//u', $this->Maj, null, PREG_SPLIT_NO_EMPTY); + $num = preg_split('//u', $this->Nb, null, PREG_SPLIT_NO_EMPTY);; + $spe = preg_split('//u', $this->Spe, null, PREG_SPLIT_NO_EMPTY); + /* $password_a = str_split($password); $maj = str_split($this->Maj); $num = str_split($this->Nb); $spe = str_split($this->Spe); + */ + + if (dol_strlen($password) < $this->length2) { + $langs->load("other"); + $this->error = $langs->trans("YourPasswordMustHaveAtLeastXChars", $this->length2); + return 0; + } if (count(array_intersect($password_a, $maj)) < $this->NbMaj) { - return false; + $langs->load("other"); + $this->error = $langs->trans('PasswordNeedAtLeastXUpperCaseChars', $this->NbMaj); + return 0; } if (count(array_intersect($password_a, $num)) < $this->NbNum) { - return false; + $langs->load("other"); + $this->error = $langs->trans('PasswordNeedAtLeastXDigitChars', $this->NbNum); + return 0; } if (count(array_intersect($password_a, $spe)) < $this->NbSpe) { - return false; + $langs->load("other"); + $this->error = $langs->trans('PasswordNeedAtLeastXSpecialChars', $this->NbSpe); + return 0; } - if (!$this->consecutiveInterationSameCharacter($password)) { - return false; + if (!$this->consecutiveIterationSameCharacter($password)) { + $langs->load("other"); + $this->error = $langs->trans('PasswordNeedNoXConsecutiveChars', $this->NbRepeat); + return 0; } - return true; + return 1; } /** - * Check the consecutive iterations of the same character. Return false if the number doesn't match the maximum consecutive value allowed. + * Check the consecutive iterations of the same character. * * @param string $password Password to check - * @return bool + * @return bool False if the number doesn't match the maximum consecutive value allowed. */ - private function consecutiveInterationSameCharacter($password) + public function consecutiveIterationSameCharacter($password) { - $last = ""; + $this->initAll(); if (empty($this->NbRepeat)) { - return 1; + return true; } - $count = 0; - $char = str_split($password); + $char = preg_split('//u', $password, null, PREG_SPLIT_NO_EMPTY); + $last = ""; + $count = 0; foreach ($char as $c) { if ($c != $last) { $last = $c; - $count = 0; - + $count = 1; + //print "Char $c - count = $count\n"; continue; } $count++; + //print "Char $c - count = $count\n"; + if ($count > $this->NbRepeat) { return false; } diff --git a/htdocs/core/modules/security/generate/modGeneratePassStandard.class.php b/htdocs/core/modules/security/generate/modGeneratePassStandard.class.php index 1861cc9bb90..f0e6fcf2a7f 100644 --- a/htdocs/core/modules/security/generate/modGeneratePassStandard.class.php +++ b/htdocs/core/modules/security/generate/modGeneratePassStandard.class.php @@ -137,15 +137,21 @@ class modGeneratePassStandard extends ModeleGenPassword /** * Validate a password + * This function is called by User->setPassword() and internally to validate that the password matches the constraints. * * @param string $password Password to check * @return int 0 if KO, >0 if OK */ public function validatePassword($password) { - if (dol_strlen($password) < $this->length) { + global $langs; + + if (dol_strlen($password) < $this->length2) { + $langs->load("other"); + $this->error = $langs->trans("YourPasswordMustHaveAtLeastXChars", $this->length2); return 0; } + return 1; } } diff --git a/htdocs/core/modules/security/generate/modules_genpassword.php b/htdocs/core/modules/security/generate/modules_genpassword.php index 40bebc0fb2d..4d397605240 100644 --- a/htdocs/core/modules/security/generate/modules_genpassword.php +++ b/htdocs/core/modules/security/generate/modules_genpassword.php @@ -29,6 +29,13 @@ require_once DOL_DOCUMENT_ROOT.'/core/lib/functions.lib.php'; */ abstract class ModeleGenPassword { + /** + * Flag to 1 if we must clean ambiguous charaters for the autogeneration of password (List of ambiguous char is in $this->Ambi) + * + * @var integer + */ + public $WithoutAmbi = 0; + /** * @var string Error code (or message) */ @@ -79,10 +86,11 @@ abstract class ModeleGenPassword } /** - * Validate a password + * Validate a password. + * This function is called by User->setPassword() and internally to validate that the password matches the constraints. * - * @param string $password Password to check - * @return int 0 if KO, >0 if OK + * @param string $password Password to check + * @return int 0 if KO, >0 if OK */ public function validatePassword($password) { diff --git a/htdocs/langs/en_US/other.lang b/htdocs/langs/en_US/other.lang index 9bc68dc3678..91dd5ba4509 100644 --- a/htdocs/langs/en_US/other.lang +++ b/htdocs/langs/en_US/other.lang @@ -258,6 +258,10 @@ PassEncoding=Password encoding PermissionsAdd=Permissions added PermissionsDelete=Permissions removed YourPasswordMustHaveAtLeastXChars=Your password must have at least %s chars +PasswordNeedAtLeastXUpperCaseChars=The password need at least %s upper case chars +PasswordNeedAtLeastXDigitChars=The password need at least %s numeric chars +PasswordNeedAtLeastXSpecialChars=The password need at least %s special chars +PasswordNeedNoXConsecutiveChars=The password must not have %s consecutive similar chars YourPasswordHasBeenReset=Your password has been reset successfully ApplicantIpAddress=IP address of applicant SMSSentTo=SMS sent to %s diff --git a/htdocs/user/class/user.class.php b/htdocs/user/class/user.class.php index a5f7a4c30aa..780744b4c8e 100644 --- a/htdocs/user/class/user.class.php +++ b/htdocs/user/class/user.class.php @@ -2130,39 +2130,21 @@ class User extends CommonObject if (!empty($conf->global->USER_PASSWORD_GENERATED)) { // Add a check on rules for password syntax using the setup of the password generator $modGeneratePassClass = 'modGeneratePass'.ucfirst($conf->global->USER_PASSWORD_GENERATED); - /* - include_once DOl_DOCUMENT_ROOT.'/core/modules/security/generate/'.$modGeneratePassClass.'.class.php'; + + include_once DOL_DOCUMENT_ROOT.'/core/modules/security/generate/'.$modGeneratePassClass.'.class.php'; if (class_exists($modGeneratePassClass)) { $modGeneratePass = new $modGeneratePassClass($this->db, $conf, $langs, $user); - // Check length - if (property_exists($modGeneratePass, 'length2') && $modGeneratePass->length2 > 0) { - if (strlen($password) < $modGeneratePass->length2) { - $this->error = "PasswordMustHaveNCharMin"; - return -1; - } - } + // To check an input user password, we disable the cleaning on ambiguous characters (this is used only for auto-generated password) + $modGeneratePass->WithoutAmbi = 0; - // Check on $modGeneratePass->NbMaj - if (property_exists($modGeneratePass, 'NbMaj') && $modGeneratePass->NbMaj > 0) { - // TODO + // Call to validatePassword($password) to check pass match rules + $testpassword = $modGeneratePass->validatePassword($password); + if (!$testpassword) { + $this->error = $modGeneratePass->error; + return -1; } - - } - // Check on $modGeneratePass->NbNum - if (property_exists($modGeneratePass, 'NbNum') && $modGeneratePass->NbNum > 0) { - // TODO - - } - // Check on $modGeneratePass->NbSpe - if (property_exists($modGeneratePass, 'NbSpe') && $modGeneratePass->NbSpe > 0) { - // TODO - } - // Check on $modGeneratePass->NbRepeat - if (property_exists($modGeneratePass, 'NbRepeat') && $modGeneratePass->NbRepeat > 0) { - // TODO - } - }*/ + } } // Now, we encrypt the new password diff --git a/test/phpunit/UserTest.php b/test/phpunit/UserTest.php index 5a09e4c9864..de92c6e62f4 100644 --- a/test/phpunit/UserTest.php +++ b/test/phpunit/UserTest.php @@ -256,15 +256,128 @@ class UserTest extends PHPUnit\Framework\TestCase print __METHOD__." localobject->date_creation=".$localobject->date_creation."\n"; $this->assertNotEquals($localobject->date_creation, ''); + return $localobject; + } + + /** + * testUserSetPassword + * + * @param User $localobject User + * @return void + * @depends testUserOther + * The depends says test is run only if previous is ok + */ + public function testUserSetPassword($localobject) + { + global $conf,$user,$langs,$db; + $conf=$this->savconf; + $user=$this->savuser; + $langs=$this->savlangs; + $db=$this->savdb; + + // Test the 'none' password generator + + $conf->global->USER_PASSWORD_GENERATED = 'none'; + + $localobject->error = ''; + $result = $localobject->setPassword($user, 'abcdef'); + print __METHOD__." set a small password with USER_PASSWORD_GENERATED = none\n"; + print __METHOD__." localobject->error=".$localobject->error."\n"; + $this->assertEquals('abcdef', $result); + + // Test the 'standard' password generator + + $conf->global->USER_PASSWORD_GENERATED = 'standard'; + + $localobject->error = ''; + $result = $localobject->setPassword($user, '12345678901'); + print __METHOD__." set a too small password with USER_PASSWORD_GENERATED = standard\n"; + print __METHOD__." localobject->error=".$localobject->error."\n"; + $this->assertEquals(-1, $result, 'We must receive a negative error code (pass too small) and we did not here'); + + // Test the 'perso' password generator + + $conf->global->USER_PASSWORD_GENERATED = 'perso'; + $conf->global->USER_PASSWORD_PATTERN = '12;2;2;2;3;1'; + + $localobject->error = ''; + $result = $localobject->setPassword($user, '12345678901'); + print __METHOD__." set a too small password with USER_PASSWORD_GENERATED = perso\n"; + print __METHOD__." localobject->error=".$localobject->error."\n"; + $this->assertEquals(-1, $result, 'We must receive a negative error code (pass too small) and we did not here'); + + $localobject->error = ''; + $result = $localobject->setPassword($user, '$*34567890AB'); + print __METHOD__." set a good password\n"; + print __METHOD__." localobject->error=".$localobject->error."\n"; + $this->assertEquals('$*34567890AB', $result, 'We must get the password as it is valid (pass enough long) and we did not here'); + + // Test uppercase : $chartofound = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; + + $localobject->error = ''; + $result = $localobject->setPassword($user, '$*123456789A'); + print __METHOD__." set a password without uppercase\n"; + print __METHOD__." localobject->error=".$localobject->error."\n"; + $this->assertEquals(-1, $result, 'We must receive a negative error code (pass without enough uppercase) and we did not here'); + + $localobject->error = ''; + $result = $localobject->setPassword($user, '$*34567890AB'); + print __METHOD__." set a password with enough uppercase\n"; + print __METHOD__." localobject->error=".$localobject->error."\n"; + $this->assertEquals('$*34567890AB', $result, 'We must get the password as it is valid (pass with enough uppercase) and we did not here'); + + // Test digits : $chartofound = "!@#$%&*()_-+={}[]\\|:;'/"; + + $localobject->error = ''; + $result = $localobject->setPassword($user, '$*ABCDEFGHIJ'); + print __METHOD__." set a password without digits\n"; + print __METHOD__." localobject->error=".$localobject->error."\n"; + $this->assertEquals(-1, $result, 'We must receive a negative error code (pass without enough digits) and we did not here'); + + $localobject->error = ''; + $result = $localobject->setPassword($user, '$*12ABCDEFGH'); + print __METHOD__." set a password with enough digits\n"; + print __METHOD__." localobject->error=".$localobject->error."\n"; + $this->assertEquals('$*12ABCDEFGH', $result, 'We must get the password as it is valid (pass with enough digits) and we did not here'); + + // Test special chars : $chartofound = "!@#$%&*()_-+={}[]\\|:;'/"; + + $localobject->error = ''; + $result = $localobject->setPassword($user, '1234567890AA'); + print __METHOD__." set a password without enough special chars\n"; + print __METHOD__." localobject->error=".$localobject->error."\n"; + $this->assertEquals(-1, $result, 'We must receive a negative error code (pass without enough special chars) and we did not here'); + + $localobject->error = ''; + $result = $localobject->setPassword($user, '$*12345678AA'); + print __METHOD__." set a password with enough special chars\n"; + print __METHOD__." localobject->error=".$localobject->error."\n"; + $this->assertEquals('$*12345678AA', $result, 'We must get the password as it is valid (pass with enough special chars) and we did not here'); + + // Test consecutive chars + $localobject->error = ''; + $result = $localobject->setPassword($user, '$*1111567890AA'); + print __METHOD__." set a password with too many consecutive chars\n"; + print __METHOD__." localobject->error=".$localobject->error."\n"; + $this->assertEquals(-1, $result, 'We must receive a negative error code (pass has n consecutive similar chars) and we did not here'); + + $localobject->error = ''; + $result = $localobject->setPassword($user, '$*11145678AA'); + print __METHOD__." set a password with noo too much consecutive chars\n"; + print __METHOD__." localobject->error=".$localobject->error."\n"; + $this->assertEquals('$*11145678AA', $result, 'We must get the password as it is valid (pass has not too much similar consecutive chars) and we did not here'); + + return $localobject->id; } + /** * testUserDelete * * @param int $id User id * @return void - * @depends testUserOther + * @depends testUserSetPassword * The depends says test is run only if previous is ok */ public function testUserDelete($id)