FIX a lot of problem with rules for password validation

htdocs/admin/security.php

@@ -319,7 +319,7 @@ if ($conf->global->USER_PASSWORD_GENERATED == "Perso") {
 
 	print '<tr class="oddeven">';
 	print '<td>'.$langs->trans("NoAmbiCaracAutoGeneration")."</td>";
-	print '<td><input type="checkbox" id="NoAmbiCaracAutoGeneration" '.($tabConf[5] ? "checked" : "").' min="0"> <span id="textcheckbox">'.($tabConf[5] ? $langs->trans("Activated") : $langs->trans("Disabled")).'</span></td>';
+	print '<td><input type="checkbox" id="NoAmbiCaracAutoGeneration" '.($tabConf[5] ? "checked" : "").' min="0"> <label for="NoAmbiCaracAutoGeneration" id="textcheckbox">'.($tabConf[5] ? $langs->trans("Activated") : $langs->trans("Disabled")).'</label></td>';
 	print '</tr>';
 
 	print '</table>';

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
+If you want to add your own password generator module. This is steps to follow to add your own password generator:
-to add you 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
+You can choose value of your choice instead of "Mygenerator" in name of new file.
-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"
+"class modGeneratePassNone"     into    "class modGeneratePassMygenerator"
-"function modGeneratePassNone"    into    "function modMyGenerator"
 
 Then add code inside the "getDecription" function. 
 Then add code inside the "getExample" function. 

htdocs/core/modules/security/generate/modGeneratePassNone.class.php

@@ -111,7 +111,8 @@ 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

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,10 +133,6 @@ 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;
 	}
 
 	/**
@@ -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)) {
+		if (!$this->consecutiveIterationSameCharacter($password)) {
-			return false;
+			$langs->load("other");
+			$this->error = $langs->trans('PasswordNeedNoXConsecutiveChars', $this->NbRepeat);
+			return 0;
 		}
 
-		return true;
-	}
-
-	/**
-	 *  Check the consecutive iterations of the same character. Return false if the number doesn't match the maximum consecutive value allowed.
-	 *
-	 *  @param		string	$password	Password to check
-	 *  @return     bool
-	 */
-	private function consecutiveInterationSameCharacter($password)
-	{
-		$last = "";
-
-		if (empty($this->NbRepeat)) {
 		return 1;
 	}
 
-		$count = 0;
+	/**
-		$char = str_split($password);
+	 *  Check the consecutive iterations of the same character.
+	 *
+	 *  @param		string	$password	Password to check
+	 *  @return     bool				False if the number doesn't match the maximum consecutive value allowed.
+	 */
+	public function consecutiveIterationSameCharacter($password)
+	{
+		$this->initAll();
 
+		if (empty($this->NbRepeat)) {
+			return true;
+		}
+
+		$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;
 			}

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;
 	}
 }

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,7 +86,8 @@ 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

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 <strong>%s</strong> chars
+PasswordNeedAtLeastXUpperCaseChars=The password need at least <strong>%s</strong> upper case chars 
+PasswordNeedAtLeastXDigitChars=The password need at least <strong>%s</strong> numeric chars 
+PasswordNeedAtLeastXSpecialChars=The password need at least <strong>%s</strong> special chars
+PasswordNeedNoXConsecutiveChars=The password must not have <strong>%s</strong> consecutive similar chars 
 YourPasswordHasBeenReset=Your password has been reset successfully
 ApplicantIpAddress=IP address of applicant
 SMSSentTo=SMS sent to %s

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
+					// To check an input user password, we disable the cleaning on ambiguous characters (this is used only for auto-generated password)
-					if (property_exists($modGeneratePass, 'length2') && $modGeneratePass->length2 > 0) {
+					$modGeneratePass->WithoutAmbi = 0;
-						if (strlen($password) < $modGeneratePass->length2) {
+
-							$this->error = "PasswordMustHaveNCharMin";
+					// Call to validatePassword($password) to check pass match rules
+					$testpassword = $modGeneratePass->validatePassword($password);
+					if (!$testpassword) {
+						$this->error = $modGeneratePass->error;
 						return -1;
 					}
 				}
-
-					// Check on $modGeneratePass->NbMaj
-					if (property_exists($modGeneratePass, 'NbMaj') && $modGeneratePass->NbMaj > 0) {
-						// TODO
-					}
-
-					}
-					// 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

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)