Skip to content

Commit

Permalink
Fix user_dn_hash reconciliation when user DN contains special chars
Browse files Browse the repository at this point in the history 
fixes #16725
  • Loading branch information
@cedric-anne
cedric-anne committed Mar 19, 2024
1 parent 36e536b commit 93a9f69
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 6 deletions.
72 changes: 72 additions & 0 deletions install/migrations/update_10.0.14_to_10.0.15.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
<?php

/**
* ---------------------------------------------------------------------
*
* GLPI - Gestionnaire Libre de Parc Informatique
*
* http://glpi-project.org
*
* @copyright 2015-2024 Teclib' and contributors.
* @copyright 2003-2014 by the INDEPNET Development Team.
* @licence https://www.gnu.org/licenses/gpl-3.0.html
*
* ---------------------------------------------------------------------
*
* LICENSE
*
* This file is part of GLPI.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*
* ---------------------------------------------------------------------
*/

/**
* 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;
}
66 changes: 66 additions & 0 deletions install/migrations/update_10.0.14_to_10.0.15/user.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
<?php

/**
* ---------------------------------------------------------------------
*
* GLPI - Gestionnaire Libre de Parc Informatique
*
* http://glpi-project.org
*
* @copyright 2015-2024 Teclib' and contributors.
* @copyright 2003-2014 by the INDEPNET Development Team.
* @licence https://www.gnu.org/licenses/gpl-3.0.html
*
* ---------------------------------------------------------------------
*
* LICENSE
*
* This file is part of GLPI.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*
* ---------------------------------------------------------------------
*/

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'],
]
)
);
}
10 changes: 7 additions & 3 deletions src/User.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
]);
}

Expand Down Expand Up @@ -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']));
}
}

Expand Down Expand Up @@ -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;
}
}

Expand Down
8 changes: 5 additions & 3 deletions tests/functional/User.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1333,15 +1333,15 @@ 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
]);
$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
]);
Expand Down Expand Up @@ -1380,14 +1380,15 @@ 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
]);

// 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
Expand All @@ -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();
}
Expand Down

0 comments on commit 93a9f69

Please sign in to comment.