diff --git a/install/migrations/update_10.0.11_to_10.0.12/user.php b/install/migrations/update_10.0.11_to_10.0.12/user.php new file mode 100644 index 00000000000..1bcd4215d74 --- /dev/null +++ b/install/migrations/update_10.0.11_to_10.0.12/user.php @@ -0,0 +1,59 @@ +. + * + * --------------------------------------------------------------------- + */ + +/** + * @var \DBmysql $DB + * @var \Migration $migration + */ + + // Add user_dn_hash field +$migration->addField('glpi_users', 'user_dn_hash', 'varchar(32)', [ + 'after' => 'user_dn', +]); + +$migration->addPostQuery($DB->buildUpdate( + 'glpi_users', + [ + 'user_dn_hash' => new \QueryExpression('MD5(`user_dn`)'), + ], + [ + 'NOT' => [ + 'user_dn' => null + ] + ] +)); + +// Add user_dn_hash index +$migration->addKey('glpi_users', 'user_dn_hash'); diff --git a/install/mysql/glpi-empty.sql b/install/mysql/glpi-empty.sql index a523ed903c1..20e2567b99b 100644 --- a/install/mysql/glpi-empty.sql +++ b/install/mysql/glpi-empty.sql @@ -7610,6 +7610,7 @@ CREATE TABLE `glpi_users` ( `password_forget_token` char(40) DEFAULT NULL, `password_forget_token_date` timestamp NULL DEFAULT NULL, `user_dn` text, + `user_dn_hash` varchar(32), `registration_number` varchar(255) DEFAULT NULL, `show_count_on_tabs` tinyint DEFAULT NULL, `refresh_views` int DEFAULT NULL, @@ -7686,7 +7687,8 @@ CREATE TABLE `glpi_users` ( KEY `groups_id` (`groups_id`), KEY `users_id_supervisor` (`users_id_supervisor`), KEY `auths_id` (`auths_id`), - KEY `default_requesttypes_id` (`default_requesttypes_id`) + KEY `default_requesttypes_id` (`default_requesttypes_id`), + KEY `user_dn_hash` (`user_dn_hash`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci ROW_FORMAT=DYNAMIC; diff --git a/src/User.php b/src/User.php index e00acc4131c..17ac1b493d2 100644 --- a/src/User.php +++ b/src/User.php @@ -550,15 +550,20 @@ public function getFromDBbySyncField($value) /** * Retrieve a user from the database using it's dn. * - * @since 0.84 - * * @param string $user_dn dn of the user * * @return boolean */ public function getFromDBbyDn($user_dn) { - return $this->getFromDBByCrit(['user_dn' => $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) + ]); } /** @@ -833,6 +838,14 @@ public function computeCloneName( ); } + 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']); + } + } + public function post_addItem() { @@ -1187,8 +1200,6 @@ public function post_updateItem($history = true) } } - - /** * Apply rules to determine dynamic rights of the user. * @@ -3483,6 +3494,12 @@ public function pre_updateInDB() unset($this->oldvalues['comment']); } } + + // 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; + } } public function getSpecificMassiveActions($checkitem = null) diff --git a/tests/LDAP/AuthLdap.php b/tests/LDAP/AuthLdap.php index 06f86448ecf..3b751910093 100644 --- a/tests/LDAP/AuthLdap.php +++ b/tests/LDAP/AuthLdap.php @@ -1336,6 +1336,7 @@ public function testLdapAuth() unset($dup['id']); unset($dup['date_creation']); unset($dup['date_mod']); + unset($dup['user_dn_hash']); $aid = $dup['auths_id']; $dup['auths_id'] = $aid + 1; diff --git a/tests/functional/User.php b/tests/functional/User.php index ed86ff1a342..20cea043051 100644 --- a/tests/functional/User.php +++ b/tests/functional/User.php @@ -1293,4 +1293,89 @@ public function testUserPreferences() $this->variable($user->fields['show_count_on_tabs'])->isNull(); $this->variable($user->fields['itil_layout'])->isEqualTo($itil_layout_2); } + + /** + * Test that user_dn_hash is correctly set on user creation and update + * + * @return void + */ + public function testUserDnIsHashedOnAddAndUpdate(): void + { + // Create user whithout dn and check that user_dn_hash is not set + $user = $this->createItem('User', [ + 'name' => __FUNCTION__, + ]); + $this->variable($user->fields['user_dn'])->isNull(); + $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'; + $user = $this->createItem('User', [ + 'name' => __FUNCTION__ . '_created', + 'user_dn' => $dn + ]); + $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'; + $this->updateItem('User', $user->getID(), [ + 'user_dn' => $dn + ]); + $user->getFromDB($user->getID()); + $this->string($user->fields['user_dn_hash'])->isEqualTo(md5($dn)); + + // Set user_dn to empty and check that user_dn_hash is set to null + $this->updateItem('User', $user->getID(), [ + 'user_dn' => '' + ]); + $user->getFromDB($user->getID()); + $this->variable($user->fields['user_dn_hash'])->isNull(); + + // Set user_dn to null and check that user_dn_hash is set to null + $this->updateItem('User', $user->getID(), [ + 'user_dn' => null + ]); + $user->getFromDB($user->getID()); + $this->variable($user->fields['user_dn_hash'])->isNull(); + } + + /** + * Test that user_dn_hash is correctly used in getFromDBbyDn method + * + * @return void + */ + public function testUserDnHashIsUsedInGetFromDBbyDn(): void + { + global $DB; + + $retrievedUser = new \User(); + + // Get a user with a bad dn + $this->boolean($retrievedUser->getFromDBbyDn(__FUNCTION__)) + ->isFalse(); + $this->boolean($retrievedUser->isNewItem())->isTrue(); + + // Create a user with a dn + $dn = 'user=' . __FUNCTION__ . ',dc=test,dc=glpi-project,dc=org'; + $user = $this->createItem('User', [ + 'name' => __FUNCTION__, + 'user_dn' => $dn + ]); + + // Retrieve the user using getFromDBbyDn method + $this->boolean($retrievedUser->getFromDBbyDn($dn))->isTrue(); + $this->boolean($retrievedUser->isNewItem())->isFalse(); + + // Unset user_dn to check that user_dn_hash is used + $DB->update( + \User::getTable(), + ['user_dn' => ''], + ['id' => $user->getID()] + ); + + // Retrieve the user using getFromDBbyDn and check if user_dn_hash is used + $this->boolean($retrievedUser->getFromDBbyDn($dn))->isTrue(); + $this->boolean($retrievedUser->isNewItem())->isFalse(); + $this->string($retrievedUser->fields['user_dn'])->isEmpty(); + } }