From 6bfef21d080531427ba6ca67b15643ab50e1218d Mon Sep 17 00:00:00 2001 From: Julio Montoya Date: Thu, 17 Jun 2021 13:40:39 +0200 Subject: [PATCH] Move constant USERNAME_MAX_LENGTH in the user entity Remove unused code + fix isPasswordValid using symfony hasher --- public/main/admin/configure_inscription.php | 4 +- public/main/admin/user_add.php | 13 +-- public/main/admin/user_edit.php | 11 +-- public/main/auth/inscription.php | 13 +-- public/main/auth/profile.php | 7 +- .../main/gradebook/lib/scoredisplay.class.php | 9 +- public/main/inc/global.inc.php | 1 - public/main/inc/lib/certificate.lib.php | 29 ------- public/main/inc/lib/usermanager.lib.php | 87 +++---------------- .../inc/lib/webservices/WebService.class.php | 5 +- public/main/install/install.lib.php | 1 - .../whispeakauth/ajax/authentify_password.php | 2 +- src/CoreBundle/Entity/User.php | 1 + 13 files changed, 48 insertions(+), 135 deletions(-) diff --git a/public/main/admin/configure_inscription.php b/public/main/admin/configure_inscription.php index 00c3e34168..94e805d665 100644 --- a/public/main/admin/configure_inscription.php +++ b/public/main/admin/configure_inscription.php @@ -185,11 +185,11 @@ if ($display_all_form) { $form->addEmailRule('email'); // USERNAME - $form->addElement('text', 'username', get_lang('Username'), ['size' => USERNAME_MAX_LENGTH, 'disabled' => 'disabled']); + $form->addElement('text', 'username', get_lang('Username'), ['size' => \Chamilo\CoreBundle\Entity\User::USERNAME_MAX_LENGTH, 'disabled' => 'disabled']); $form->addRule('username', get_lang('Required field'), 'required'); $form->addRule('username', get_lang('Your login can only contain letters, numbers and _.-'), 'username'); $form->addRule('username', get_lang('This login is already in use'), 'username_available'); - $form->addRule('username', sprintf(get_lang('The login needs to be maximum %s characters long'), (string) USERNAME_MAX_LENGTH), 'maxlength', USERNAME_MAX_LENGTH); + $form->addRule('username', sprintf(get_lang('The login needs to be maximum %s characters long'), (string) \Chamilo\CoreBundle\Entity\User::USERNAME_MAX_LENGTH), 'maxlength', \Chamilo\CoreBundle\Entity\User::USERNAME_MAX_LENGTH); // PASSWORD $form->addElement('password', 'pass1', get_lang('Pass'), ['size' => 40, 'disabled' => 'disabled']); diff --git a/public/main/admin/user_add.php b/public/main/admin/user_add.php index 1c43be32e6..e3446edd58 100644 --- a/public/main/admin/user_add.php +++ b/public/main/admin/user_add.php @@ -2,6 +2,7 @@ /* For licensing terms, see /license.txt */ +use Chamilo\CoreBundle\Entity\User; use Chamilo\CoreBundle\Framework\Container; $cidReset = true; @@ -141,12 +142,12 @@ if ('true' == api_get_setting('registration', 'email')) { $form->addRule('email', get_lang('Required field'), 'required'); } -if ('true' == api_get_setting('login_is_email')) { +if ('true' === api_get_setting('login_is_email')) { $form->addRule( 'email', - sprintf(get_lang('The login needs to be maximum %s characters long'), (string) USERNAME_MAX_LENGTH), + sprintf(get_lang('The login needs to be maximum %s characters long'), (string) User::USERNAME_MAX_LENGTH), 'maxlength', - USERNAME_MAX_LENGTH + User::USERNAME_MAX_LENGTH ); $form->addRule('email', get_lang('This login is already in use'), 'username_available'); } @@ -164,10 +165,10 @@ $allowed_picture_types = api_get_supported_image_extensions(false); $form->addRule('picture', get_lang('Only PNG, JPG or GIF images allowed').' ('.implode(',', $allowed_picture_types).')', 'filetype', $allowed_picture_types); // Username -if ('true' != api_get_setting('login_is_email')) { - $form->addElement('text', 'username', get_lang('Login'), ['id' => 'username', 'maxlength' => USERNAME_MAX_LENGTH, 'autocomplete' => 'off']); +if ('true' !== api_get_setting('login_is_email')) { + $form->addElement('text', 'username', get_lang('Login'), ['id' => 'username', 'maxlength' => User::USERNAME_MAX_LENGTH, 'autocomplete' => 'off']); $form->addRule('username', get_lang('Required field'), 'required'); - $form->addRule('username', sprintf(get_lang('The login needs to be maximum %s characters long'), (string) USERNAME_MAX_LENGTH), 'maxlength', USERNAME_MAX_LENGTH); + $form->addRule('username', sprintf(get_lang('The login needs to be maximum %s characters long'), (string) User::USERNAME_MAX_LENGTH), 'maxlength', User::USERNAME_MAX_LENGTH); $form->addRule('username', get_lang('Only letters and numbers allowed'), 'username'); $form->addRule('username', get_lang('This login is already in use'), 'username_available'); } diff --git a/public/main/admin/user_edit.php b/public/main/admin/user_edit.php index a10e06ab88..27624fc6ff 100644 --- a/public/main/admin/user_edit.php +++ b/public/main/admin/user_edit.php @@ -2,6 +2,7 @@ /* For licensing terms, see /license.txt */ +use Chamilo\CoreBundle\Entity\User; use Chamilo\CoreBundle\Framework\Container; use ChamiloSession as Session; @@ -143,9 +144,9 @@ if ('true' == api_get_setting('registration', 'email')) { if ('true' == api_get_setting('login_is_email')) { $form->addRule( 'email', - sprintf(get_lang('The login needs to be maximum %s characters long'), (string) USERNAME_MAX_LENGTH), + sprintf(get_lang('The login needs to be maximum %s characters long'), (string) User::USERNAME_MAX_LENGTH), 'maxlength', - USERNAME_MAX_LENGTH + User::USERNAME_MAX_LENGTH ); $form->addRule('email', get_lang('This login is already in use'), 'username_available', $user_data['username']); } @@ -176,13 +177,13 @@ if ($hasPicture) { // Username if ('true' !== api_get_setting('login_is_email')) { - $form->addElement('text', 'username', get_lang('Login'), ['maxlength' => USERNAME_MAX_LENGTH]); + $form->addElement('text', 'username', get_lang('Login'), ['maxlength' => User::USERNAME_MAX_LENGTH]); $form->addRule('username', get_lang('Required field'), 'required'); $form->addRule( 'username', - sprintf(get_lang('The login needs to be maximum %s characters long'), (string) USERNAME_MAX_LENGTH), + sprintf(get_lang('The login needs to be maximum %s characters long'), (string) User::USERNAME_MAX_LENGTH), 'maxlength', - USERNAME_MAX_LENGTH + User::USERNAME_MAX_LENGTH ); $form->addRule('username', get_lang('Only letters and numbers allowed'), 'username'); $form->addRule('username', get_lang('This login is already in use'), 'username_available', $user_data['username']); diff --git a/public/main/auth/inscription.php b/public/main/auth/inscription.php index d876287598..f5f4bd7a86 100644 --- a/public/main/auth/inscription.php +++ b/public/main/auth/inscription.php @@ -1,6 +1,7 @@ addRule('email', get_lang('This login is already in use'), 'username_available'); } @@ -171,7 +172,7 @@ if (false === $user_already_registered_show_terms && true, [ 'id' => 'username', - 'size' => USERNAME_MAX_LENGTH, + 'size' => User::USERNAME_MAX_LENGTH, 'autocomplete' => 'off', ] ); @@ -181,10 +182,10 @@ if (false === $user_already_registered_show_terms && 'username', sprintf( get_lang('The login needs to be maximum %s characters long'), - (string) USERNAME_MAX_LENGTH + (string) User::USERNAME_MAX_LENGTH ), 'maxlength', - USERNAME_MAX_LENGTH + User::USERNAME_MAX_LENGTH ); $form->addRule('username', get_lang('Your login can only contain letters, numbers and _.-'), 'username'); $form->addRule('username', get_lang('This login is already in use'), 'username_available'); @@ -632,7 +633,7 @@ if ($form->validate()) { $values = $form->getSubmitValues(1); // Make *sure* the login isn't too long if (isset($values['username'])) { - $values['username'] = api_substr($values['username'], 0, USERNAME_MAX_LENGTH); + $values['username'] = api_substr($values['username'], 0, User::USERNAME_MAX_LENGTH); } if ('false' === api_get_setting('allow_registration_as_teacher')) { diff --git a/public/main/auth/profile.php b/public/main/auth/profile.php index aeee0442de..62b2e96d0f 100644 --- a/public/main/auth/profile.php +++ b/public/main/auth/profile.php @@ -138,8 +138,8 @@ $form->addElement( get_lang('Username'), [ 'id' => 'username', - 'maxlength' => USERNAME_MAX_LENGTH, - 'size' => USERNAME_MAX_LENGTH, + 'maxlength' => User::USERNAME_MAX_LENGTH, + 'size' => User::USERNAME_MAX_LENGTH, ] ); if (!in_array('login', $profileList) || 'true' == api_get_setting('login_is_email')) { @@ -384,9 +384,8 @@ if ($form->validate()) { ) { $passwordWasChecked = true; $validPassword = UserManager::isPasswordValid( - $user->getPassword(), + $user, $user_data['password0'], - $user->getSalt() ); if ($validPassword) { diff --git a/public/main/gradebook/lib/scoredisplay.class.php b/public/main/gradebook/lib/scoredisplay.class.php index 5a0677c897..6e9ce01129 100644 --- a/public/main/gradebook/lib/scoredisplay.class.php +++ b/public/main/gradebook/lib/scoredisplay.class.php @@ -66,8 +66,8 @@ class ScoreDisplay $this->custom_display = $portal_displays; if (count($this->custom_display) > 0) { $value = api_get_setting('gradebook_score_display_upperlimit'); - $value = $value['my_display_upperlimit']; - $this->upperlimit_included = 'true' == $value ? true : false; + //$value = $value['my_display_upperlimit']; + $this->upperlimit_included = 'true' === $value ? true : false; $this->custom_display_conv = $this->convert_displays($this->custom_display); } } @@ -643,8 +643,9 @@ class ScoreDisplay $converted2 = []; foreach ($converted as $element) { $newelement = []; - if (isset($highest) && !empty($highest) && $highest > 0) { - $newelement['score'] = $element['score'] / $highest; + $highest = (float) $highest; + if (!empty($highest) && $highest > 0) { + $newelement['score'] = (float) $element['score'] / $highest; } else { $newelement['score'] = 0; } diff --git a/public/main/inc/global.inc.php b/public/main/inc/global.inc.php index 63460ea2e8..64aaf0d7fb 100644 --- a/public/main/inc/global.inc.php +++ b/public/main/inc/global.inc.php @@ -14,7 +14,6 @@ use Symfony\Component\HttpKernel\HttpKernelInterface; /** * All legacy Chamilo scripts should include this important file. */ -define('USERNAME_MAX_LENGTH', 100); require_once __DIR__.'/../../../vendor/autoload.php'; diff --git a/public/main/inc/lib/certificate.lib.php b/public/main/inc/lib/certificate.lib.php index de8eaf77cc..a5cb349cd9 100644 --- a/public/main/inc/lib/certificate.lib.php +++ b/public/main/inc/lib/certificate.lib.php @@ -68,8 +68,6 @@ class Certificate extends Model } if ($this->user_id) { - // Need to be called before any operation - $this->check_certificate_path(); // To force certification generation if ($this->force_certificate_generation) { $this->generate([], $sendNotification); @@ -90,7 +88,6 @@ class Certificate extends Model $this->html_file = $this->certification_user_path.basename($this->certificate_data['path_certificate']); $this->qr_file = $this->certification_user_path.$pathinfo['filename'].'_qr.png'; } else { - $this->check_certificate_path(); if (api_get_configuration_value('allow_general_certificate')) { // General certificate $name = md5($this->user_id).'.html'; @@ -133,30 +130,6 @@ class Certificate extends Model } } - /** - * Checks if the certificate user path directory is created. - */ - public function check_certificate_path() - { - $this->certification_user_path = null; - - // Setting certification path - $path_info = UserManager::getUserPathById($this->user_id, 'system'); - $web_path_info = UserManager::getUserPathById($this->user_id, 'web'); - - if (!empty($path_info) && isset($path_info)) { - $this->certification_user_path = $path_info.'certificate/'; - $this->certification_web_user_path = $web_path_info.'certificate/'; - $mode = api_get_permissions_for_new_directories(); - if (!is_dir($path_info)) { - mkdir($path_info, $mode, true); - } - if (!is_dir($this->certification_user_path)) { - mkdir($this->certification_user_path, $mode); - } - } - } - /** * Deletes the current certificate object. This is generally triggered by * the teacher from the gradebook tool to re-generate the certificate because @@ -328,8 +301,6 @@ class Certificate extends Model } } } else { - $this->check_certificate_path(); - // General certificate $name = md5($this->user_id).'.html'; $my_path_certificate = $this->certification_user_path.$name; diff --git a/public/main/inc/lib/usermanager.lib.php b/public/main/inc/lib/usermanager.lib.php index 78e94b861e..6dbb649f41 100644 --- a/public/main/inc/lib/usermanager.lib.php +++ b/public/main/inc/lib/usermanager.lib.php @@ -39,13 +39,6 @@ class UserManager public const USER_FIELD_TYPE_FILE = 13; public const USER_FIELD_TYPE_MOBILE_PHONE_NUMBER = 14; - private static $encryptionMethod; - - /** - * Constructor. - * - * @assert () === null - */ public function __construct() { } @@ -60,42 +53,16 @@ class UserManager return Container::getUserRepository(); } - /** - * @param string $encryptionMethod - */ - public static function setPasswordEncryption($encryptionMethod) - { - self::$encryptionMethod = $encryptionMethod; - } - - /** - * @return bool|mixed - */ - public static function getPasswordEncryption() - { - $encryptionMethod = self::$encryptionMethod; - if (empty($encryptionMethod)) { - $encryptionMethod = api_get_configuration_value('password_encryption'); - } - - return $encryptionMethod; - } - /** * Validates the password. * - * @param $encoded - * @param $raw - * @param $salt - * * @return bool */ - public static function isPasswordValid($encoded, $raw, $salt) + public static function isPasswordValid(User $user, $plainPassword) { - $encoder = new \Chamilo\CoreBundle\Security\Encoder(self::getPasswordEncryption()); - $validPassword = $encoder->isPasswordValid($encoded, $raw, $salt); + $hasher = Container::$container->get('security.user_password_hasher'); - return $validPassword; + return $hasher->isPasswordValid($user, $plainPassword); } /** @@ -1382,7 +1349,7 @@ class UserManager $username = URLify::transliterate($username); - return strtolower(substr($username, 0, USERNAME_MAX_LENGTH - 3)); + return strtolower(substr($username, 0, User::USERNAME_MAX_LENGTH - 3)); } /** @@ -1414,10 +1381,10 @@ class UserManager } if (!self::is_username_available($username)) { $i = 2; - $temp_username = substr($username, 0, USERNAME_MAX_LENGTH - strlen((string) $i)).$i; + $temp_username = substr($username, 0, User::USERNAME_MAX_LENGTH - strlen((string) $i)).$i; while (!self::is_username_available($temp_username)) { $i++; - $temp_username = substr($username, 0, USERNAME_MAX_LENGTH - strlen((string) $i)).$i; + $temp_username = substr($username, 0, User::USERNAME_MAX_LENGTH - strlen((string) $i)).$i; } $username = $temp_username; } @@ -1443,7 +1410,7 @@ class UserManager // into ASCII letters in order they not to be totally removed. // 2. Applying the strict purifier. // 3. Length limitation. - $return = 'true' === api_get_setting('login_is_email') ? substr(preg_replace(USERNAME_PURIFIER_MAIL, '', $username), 0, USERNAME_MAX_LENGTH) : substr(preg_replace(USERNAME_PURIFIER, '', $username), 0, USERNAME_MAX_LENGTH); + $return = 'true' === api_get_setting('login_is_email') ? substr(preg_replace(USERNAME_PURIFIER_MAIL, '', $username), 0, User::USERNAME_MAX_LENGTH) : substr(preg_replace(USERNAME_PURIFIER, '', $username), 0, User::USERNAME_MAX_LENGTH); $return = URLify::transliterate($return); return $return; @@ -1454,7 +1421,7 @@ class UserManager return substr( preg_replace(USERNAME_PURIFIER_SHALLOW, '', $username), 0, - USERNAME_MAX_LENGTH + User::USERNAME_MAX_LENGTH ); } @@ -1521,7 +1488,7 @@ class UserManager */ public static function is_username_too_long($username) { - return strlen($username) > USERNAME_MAX_LENGTH; + return strlen($username) > User::USERNAME_MAX_LENGTH; } /** @@ -2246,7 +2213,6 @@ class UserManager $extra_data = []; $t_uf = Database::get_main_table(TABLE_EXTRA_FIELD); $t_ufv = Database::get_main_table(TABLE_EXTRA_FIELD_VALUES); - $user_id = (int) $user_id; $sql = "SELECT f.id as id, f.variable as fvar, f.field_type as type FROM $t_uf f WHERE @@ -3253,7 +3219,7 @@ class UserManager * * @return string */ - public static function get_user_upload_files_by_course( + /*public static function get_user_upload_files_by_course( $user_id, $course, $resourceType = 'all' @@ -3302,7 +3268,7 @@ class UserManager } return $return; - } + }*/ /** * Gets the API key (or keys) and return them into an array. @@ -3529,8 +3495,8 @@ class UserManager WHERE 1 = 1 "; } - if (is_int($status) && $status > 0) { - $status = (int) $status; + $status = (int) $status; + if (!empty($status) && $status > 0) { $sql .= " AND u.status = $status "; } @@ -5761,7 +5727,7 @@ SQL; ->setLastname($uniqueId) ->setBiography('') ->setAddress('') - ->setCurriculumItems(null) + //->setCurriculumItems(null) ->setDateOfBirth(null) ->setCompetences('') ->setDiplomas('') @@ -6295,31 +6261,6 @@ SQL; return Database::num_rows($result) > 0; } - /** - * @return EncoderFactory - */ - private static function getEncoderFactory() - { - $encryption = self::getPasswordEncryption(); - $encoders = [ - 'Chamilo\\CoreBundle\\Entity\\User' => new \Chamilo\CoreBundle\Security\Encoder($encryption), - ]; - - $encoderFactory = new \Symfony\Component\Security\Core\Encoder\EncoderFactory($encoders); - - return $encoderFactory; - } - - /** - * @return \Symfony\Component\Security\Core\Encoder\PasswordEncoderInterface - */ - private static function getEncoder(User $user) - { - $encoderFactory = self::getEncoderFactory(); - - return $encoderFactory->getEncoder($user); - } - /** * Disables or enables a user. * diff --git a/public/main/inc/lib/webservices/WebService.class.php b/public/main/inc/lib/webservices/WebService.class.php index 195690d48f..7b9f3e86a0 100644 --- a/public/main/inc/lib/webservices/WebService.class.php +++ b/public/main/inc/lib/webservices/WebService.class.php @@ -93,9 +93,8 @@ class WebService } return UserManager::isPasswordValid( - $user->getPassword(), - $password, - $user->getSalt() + $user, + $password ); } diff --git a/public/main/install/install.lib.php b/public/main/install/install.lib.php index 9ca6c932ca..4db55073ec 100644 --- a/public/main/install/install.lib.php +++ b/public/main/install/install.lib.php @@ -32,7 +32,6 @@ use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; * of older versions before upgrading. */ define('SYSTEM_CONFIG_FILENAME', 'configuration.dist.php'); -define('USERNAME_MAX_LENGTH', 100); /** * This function detects whether the system has been already installed. diff --git a/public/plugin/whispeakauth/ajax/authentify_password.php b/public/plugin/whispeakauth/ajax/authentify_password.php index ef75d7b675..8f4e69d1b9 100644 --- a/public/plugin/whispeakauth/ajax/authentify_password.php +++ b/public/plugin/whispeakauth/ajax/authentify_password.php @@ -42,7 +42,7 @@ $lpItemInfo = ChamiloSession::read(WhispeakAuthPlugin::SESSION_LP_ITEM, []); /** @var array $quizQuestionInfo */ $quizQuestionInfo = ChamiloSession::read(WhispeakAuthPlugin::SESSION_QUIZ_QUESTION, []); -$isValidPassword = UserManager::isPasswordValid($user->getPassword(), $password, $user->getSalt()); +$isValidPassword = UserManager::isPasswordValid($user, $password); $isActive = $user->isActive(); $isExpired = empty($user->getExpirationDate()) || $user->getExpirationDate() > api_get_utc_datetime(null, false, true); diff --git a/src/CoreBundle/Entity/User.php b/src/CoreBundle/Entity/User.php index b9a1bac59a..b40e36292b 100644 --- a/src/CoreBundle/Entity/User.php +++ b/src/CoreBundle/Entity/User.php @@ -71,6 +71,7 @@ class User implements UserInterface, EquatableInterface, ResourceInterface, Reso use TimestampableEntity; use UserCreatorTrait; + public const USERNAME_MAX_LENGTH = 100; public const ROLE_DEFAULT = 'ROLE_USER'; public const ROLE_SUPER_ADMIN = 'ROLE_SUPER_ADMIN'; public const COURSE_MANAGER = 1;