From 93a9f69bb24a71130b3f005c5123f639ff87cdc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Anne?= Date: Tue, 19 Mar 2024 13:23:33 +0100 Subject: [PATCH] Fix `user_dn_hash` reconciliation when user DN contains special chars fixes #16725 --- .../migrations/update_10.0.14_to_10.0.15.php | 72 +++++++++++++++++++ .../update_10.0.14_to_10.0.15/user.php | 66 +++++++++++++++++ src/User.php | 10 ++- tests/functional/User.php | 8 ++- 4 files changed, 150 insertions(+), 6 deletions(-) create mode 100644 install/migrations/update_10.0.14_to_10.0.15.php create mode 100644 install/migrations/update_10.0.14_to_10.0.15/user.php diff --git a/install/migrations/update_10.0.14_to_10.0.15.php b/install/migrations/update_10.0.14_to_10.0.15.php new file mode 100644 index 00000000000..032dee7dcd8 --- /dev/null +++ b/install/migrations/update_10.0.14_to_10.0.15.php @@ -0,0 +1,72 @@ +. + * + * --------------------------------------------------------------------- + */ + +/** + * Update from 10.0.14 to 10.0.15 + * + * @return bool for success (will die for most error) + **/ +function update10014to10015() +{ + /** + * @var \DBmysql $DB + * @var \Migration $migration + */ + global $DB, $migration; + + $updateresult = true; + $ADDTODISPLAYPREF = []; + $DELFROMDISPLAYPREF = []; + $update_dir = __DIR__ . '/update_10.0.14_to_10.0.15/'; + + //TRANS: %s is the number of new version + $migration->displayTitle(sprintf(__('Update to %s'), '10.0.15')); + $migration->setVersion('10.0.15'); + + $update_scripts = scandir($update_dir); + foreach ($update_scripts as $update_script) { + if (preg_match('/\.php$/', $update_script) !== 1) { + continue; + } + require $update_dir . $update_script; + } + + // ************ Keep it at the end ************** + $migration->updateDisplayPrefs($ADDTODISPLAYPREF, $DELFROMDISPLAYPREF); + + $migration->executeMigration(); + + return $updateresult; +} diff --git a/install/migrations/update_10.0.14_to_10.0.15/user.php b/install/migrations/update_10.0.14_to_10.0.15/user.php new file mode 100644 index 00000000000..09c3f966478 --- /dev/null +++ b/install/migrations/update_10.0.14_to_10.0.15/user.php @@ -0,0 +1,66 @@ +. + * + * --------------------------------------------------------------------- + */ + +use Glpi\Toolbox\Sanitizer; + +/** + * @var \DBmysql $DB + * @var \Migration $migration + */ + +// Fix user_dn_hash related to `user_dn` containing a special char +$users_iterator = $DB->request( + [ + 'SELECT' => ['id', 'user_dn'], + 'FROM' => 'glpi_users', + 'WHERE' => [ + // `user_dn` will contains a `&` char if any of its char was sanitized + 'user_dn' => ['LIKE', '%&%'], + ] + ] +); +foreach ($users_iterator as $user_data) { + $migration->addPostQuery( + $DB->buildUpdate( + 'glpi_users', + [ + 'user_dn_hash' => md5(Sanitizer::decodeHtmlSpecialChars($user_data['user_dn'])), + ], + [ + 'id' => $user_data['id'], + ] + ) + ); +} diff --git a/src/User.php b/src/User.php index 695fe4df5fc..0b926e379c4 100644 --- a/src/User.php +++ b/src/User.php @@ -561,13 +561,15 @@ public function getFromDBbySyncField($value) */ public function getFromDBbyDn($user_dn) { + $raw_user_dn = Sanitizer::unsanitize($user_dn); + /** * We use the 'user_dn_hash' field instead of 'user_dn' for performance reasons. * The 'user_dn_hash' field is a hashed version of the 'user_dn' field * and is indexed in the database, making it faster to search. */ return $this->getFromDBByCrit([ - 'user_dn_hash' => md5($user_dn) + 'user_dn_hash' => md5($raw_user_dn) ]); } @@ -847,7 +849,7 @@ public function pre_addInDB() { // Hash user_dn if set if (isset($this->input['user_dn']) && is_string($this->input['user_dn']) && strlen($this->input['user_dn']) > 0) { - $this->input['user_dn_hash'] = md5($this->input['user_dn']); + $this->input['user_dn_hash'] = md5(Sanitizer::unsanitize($this->input['user_dn'])); } } @@ -3503,7 +3505,9 @@ public function pre_updateInDB() // Hash user_dn if is updated if (in_array('user_dn', $this->updates)) { $this->updates[] = 'user_dn_hash'; - $this->fields['user_dn_hash'] = is_string($this->input['user_dn']) && strlen($this->input['user_dn']) > 0 ? md5($this->input['user_dn']) : null; + $this->fields['user_dn_hash'] = is_string($this->input['user_dn']) && strlen($this->input['user_dn']) > 0 + ? md5(Sanitizer::unsanitize($this->input['user_dn'])) + : null; } } diff --git a/tests/functional/User.php b/tests/functional/User.php index 99124b98980..539c8718998 100644 --- a/tests/functional/User.php +++ b/tests/functional/User.php @@ -1333,7 +1333,7 @@ public function testUserDnIsHashedOnAddAndUpdate(): void $this->variable($user->fields['user_dn_hash'])->isNull(); // Create user with dn and check that user_dn_hash is set - $dn = 'user=' . __FUNCTION__ . '_created,dc=test,dc=glpi-project,dc=org'; + $dn = 'user=' . __FUNCTION__ . '_created,dc=R&D,dc=glpi-project,dc=org'; $user = $this->createItem('User', [ 'name' => __FUNCTION__ . '_created', 'user_dn' => $dn @@ -1341,7 +1341,7 @@ public function testUserDnIsHashedOnAddAndUpdate(): void $this->string($user->fields['user_dn_hash'])->isEqualTo(md5($dn)); // Update user dn and check that user_dn_hash is updated - $dn = 'user=' . __FUNCTION__ . '_updated,dc=test,dc=glpi-project,dc=org'; + $dn = 'user=' . __FUNCTION__ . '_updated,dc=R&D,dc=glpi-project,dc=org'; $this->updateItem('User', $user->getID(), [ 'user_dn' => $dn ]); @@ -1380,7 +1380,7 @@ public function testUserDnHashIsUsedInGetFromDBbyDn(): void $this->boolean($retrievedUser->isNewItem())->isTrue(); // Create a user with a dn - $dn = 'user=' . __FUNCTION__ . ',dc=test,dc=glpi-project,dc=org'; + $dn = 'user=' . __FUNCTION__ . ',dc=R&D,dc=glpi-project,dc=org'; $user = $this->createItem('User', [ 'name' => __FUNCTION__, 'user_dn' => $dn @@ -1388,6 +1388,7 @@ public function testUserDnHashIsUsedInGetFromDBbyDn(): void // Retrieve the user using getFromDBbyDn method $this->boolean($retrievedUser->getFromDBbyDn($dn))->isTrue(); + $this->boolean($retrievedUser->getFromDBbyDn(Sanitizer::sanitize($dn)))->isTrue(); // works also with sanitized value $this->boolean($retrievedUser->isNewItem())->isFalse(); // Unset user_dn to check that user_dn_hash is used @@ -1399,6 +1400,7 @@ public function testUserDnHashIsUsedInGetFromDBbyDn(): void // Retrieve the user using getFromDBbyDn and check if user_dn_hash is used $this->boolean($retrievedUser->getFromDBbyDn($dn))->isTrue(); + $this->boolean($retrievedUser->getFromDBbyDn(Sanitizer::sanitize($dn)))->isTrue(); // works also with sanitized value $this->boolean($retrievedUser->isNewItem())->isFalse(); $this->string($retrievedUser->fields['user_dn'])->isEmpty(); }