From 589571c1d239893279916020975512fe5999db34 Mon Sep 17 00:00:00 2001 From: "Romain B." <8530352+Rom1-B@users.noreply.github.com> Date: Tue, 8 Sep 2026 10:57:30 +0200 Subject: [PATCH 1/2] Fix: correct minor bugs in the import/export workflow (#656) * Fix: correct minor bugs in the import/export workflow * stan (cherry picked from commit 9793094ba58cb8ebe404bb3edfb17c75c4c5850d) --- CHANGELOG.md | 1 + ajax/injection.php | 2 +- ajax/results.php | 2 +- front/info.form.php | 2 ++ front/popup.php | 2 ++ inc/clientinjection.class.php | 13 +++++++++--- inc/info.class.php | 5 ++++- inc/softwarelicenseinjection.class.php | 28 +++++++++++++------------- inc/softwareversioninjection.class.php | 28 +++++++++++++------------- inc/userinjection.class.php | 13 +++++------- 10 files changed, 54 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d534671e..a2519eef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Fix retrieval of dropdown and multiline values containing special characters - Fix repeated User updates failing with `Data too long for column cookie_token` by excluding token fields from update payloads (backport of PR #566 from GLPI 11-compatible line) +- Fix various minor bugs in the import/export workflow (backport of PR #656 from GLPI 11-compatible line) ## [2.14.4] - 2025-11-25 diff --git a/ajax/injection.php b/ajax/injection.php index 27ce2caa..fd84aa2b 100644 --- a/ajax/injection.php +++ b/ajax/injection.php @@ -35,6 +35,6 @@ Html::header_nocache(); } -Session::checkCentralAccess(); +Session::checkRight(PluginDatainjectionClientInjection::$rightname, READ); $model = unserialize($_SESSION['datainjection']['currentmodel']); PluginDatainjectionClientInjection::showInjectionForm($model, $_SESSION['glpiactive_entity']); diff --git a/ajax/results.php b/ajax/results.php index b102d32d..1f4611f0 100644 --- a/ajax/results.php +++ b/ajax/results.php @@ -35,6 +35,6 @@ Html::header_nocache(); } -Session::checkCentralAccess(); +Session::checkRight(PluginDatainjectionClientInjection::$rightname, READ); $model = unserialize($_SESSION['datainjection']['currentmodel']); PluginDatainjectionClientInjection::showResultsForm($model); diff --git a/front/info.form.php b/front/info.form.php index 87903652..60e099ec 100644 --- a/front/info.form.php +++ b/front/info.form.php @@ -33,6 +33,8 @@ /* Update mappings */ if (isset($_POST["update"])) { + $model = new PluginDatainjectionModel(); + $model->check($_POST['models_id'], UPDATE); PluginDatainjectionInfo::manageInfos($_POST['models_id'], $_POST); } else if (isset($_POST["delete"])) { $info = new PluginDatainjectionInfo(); diff --git a/front/popup.php b/front/popup.php index a42140cb..631a75e3 100644 --- a/front/popup.php +++ b/front/popup.php @@ -34,6 +34,8 @@ switch ($_GET["popup"]) { case "preview": + $model = new PluginDatainjectionModel(); + $model->check($_GET['models_id'], READ); Html::popHeader(__('See the file', 'datainjection'), $_SERVER['PHP_SELF']); PluginDatainjectionModel::showPreviewMappings($_GET['models_id']); Html::popFooter(); diff --git a/inc/clientinjection.class.php b/inc/clientinjection.class.php index 7cac4809..c9e3524c 100644 --- a/inc/clientinjection.class.php +++ b/inc/clientinjection.class.php @@ -417,6 +417,13 @@ public static function showResultsForm(PluginDatainjectionModel $model) Html::closeForm(); } + private static function escapeCsvFormula($value) + { + if (is_string($value) && isset($value[0]) && in_array($value[0], ['=', '+', '-', '@'], true)) { + return "'" . $value; + } + return $value; + } public static function exportErrorsInCSV() { @@ -426,7 +433,7 @@ public static function exportErrorsInCSV() if (!empty($error_lines)) { $model = unserialize(PluginDatainjectionSession::getParam('currentmodel')); - $file = PLUGIN_DATAINJECTION_UPLOAD_DIR . PluginDatainjectionSession::getParam('file_name'); + $file = PLUGIN_DATAINJECTION_UPLOAD_DIR . basename(PluginDatainjectionSession::getParam('file_name')); $mappings = $model->getMappings(); $tmpfile = fopen($file, 'w'); @@ -439,11 +446,11 @@ public static function exportErrorsInCSV() //Write lines foreach ($error_lines as $line) { - fputcsv($tmpfile, $line, $model->getBackend()->getDelimiter()); + fputcsv($tmpfile, array_map([self::class, 'escapeCsvFormula'], $line), $model->getBackend()->getDelimiter()); } fclose($tmpfile); - $name = "Error-" . PluginDatainjectionSession::getParam('file_name'); + $name = "Error-" . basename(PluginDatainjectionSession::getParam('file_name')); $name = str_replace(' ', '', $name); header('Content-disposition: attachment; filename=' . $name); header('Content-Type: application/octet-stream'); diff --git a/inc/info.class.php b/inc/info.class.php index 214dd4c8..1c0af3d1 100644 --- a/inc/info.class.php +++ b/inc/info.class.php @@ -222,7 +222,10 @@ public static function manageInfos($models_id, $infos = []) } if ($id > 0) { - $info->update($info_infos); + $existing = new self(); + if ($existing->getFromDB($id) && $existing->fields['models_id'] == $models_id) { + $info->update($info_infos); + } } else { $info_infos['models_id'] = $models_id; unset($info_infos['id']); diff --git a/inc/softwarelicenseinjection.class.php b/inc/softwarelicenseinjection.class.php index c0ef1f54..c7397a62 100644 --- a/inc/softwarelicenseinjection.class.php +++ b/inc/softwarelicenseinjection.class.php @@ -159,20 +159,20 @@ public function getValueForAdditionalMandatoryFields($fields_toinject = []) return $fields_toinject; } - $query = "SELECT `id` - FROM `glpi_softwares` - WHERE `name` = '" . $fields_toinject['SoftwareLicense']['softwares_id'] . "'" . - getEntitiesRestrictRequest( - " AND", - "glpi_softwares", - "entities_id", - $fields_toinject['SoftwareLicense']['entities_id'], - true - ); - $result = $DB->doQuery($query); - - if ($DB->numrows($result) > 0) { - $id = $DB->result($result, 0, 'id'); + $where = ['name' => $fields_toinject['SoftwareLicense']['softwares_id']] + getEntitiesRestrictCriteria( + "glpi_softwares", + "entities_id", + $fields_toinject['SoftwareLicense']['entities_id'], + true + ); + $result = $DB->request([ + 'SELECT' => 'id', + 'FROM' => 'glpi_softwares', + 'WHERE' => $where, + ]); + + if (count($result) > 0) { + $id = $result->current()['id']; //Add softwares_id to the array $fields_toinject['SoftwareLicense']['softwares_id'] = $id; } else { diff --git a/inc/softwareversioninjection.class.php b/inc/softwareversioninjection.class.php index 3365c344..a4399f10 100644 --- a/inc/softwareversioninjection.class.php +++ b/inc/softwareversioninjection.class.php @@ -154,20 +154,20 @@ public function getValueForAdditionalMandatoryFields($fields_toinject = []) return $fields_toinject; } - $query = "SELECT `id` - FROM `glpi_softwares` - WHERE `name` = '" . $fields_toinject['SoftwareVersion']['softwares_id'] . "'" . - getEntitiesRestrictRequest( - " AND", - "glpi_softwares", - "entities_id", - $fields_toinject['SoftwareVersion']['entities_id'], - true - ); - $result = $DB->doQuery($query); - - if ($DB->numrows($result) > 0) { - $id = $DB->result($result, 0, 'id'); + $where = ['name' => $fields_toinject['SoftwareVersion']['softwares_id']] + getEntitiesRestrictCriteria( + "glpi_softwares", + "entities_id", + $fields_toinject['SoftwareVersion']['entities_id'], + true + ); + $result = $DB->request([ + 'SELECT' => 'id', + 'FROM' => 'glpi_softwares', + 'WHERE' => $where, + ]); + + if (count($result) > 0) { + $id = $result->current()['id']; //Add softwares_id to the array $fields_toinject['SoftwareVersion']['softwares_id'] = $id; } else { diff --git a/inc/userinjection.class.php b/inc/userinjection.class.php index 5d69c1e2..3a8a67bf 100644 --- a/inc/userinjection.class.php +++ b/inc/userinjection.class.php @@ -235,14 +235,11 @@ public function processAfterInsertOrUpdate($values, $add = true, $rights = []) } if (isset($values['User']['password']) && ($values['User']['password'] != '')) { - //We use an SQL request because updating the password is unesasy - //(self reset password process in $user->prepareInputForUpdate()) - $password = sha1(Sanitizer::unsanitize($values['User']["password"])); - - $query = "UPDATE `glpi_users` - SET `password` = '" . $password . "' - WHERE `id` = '" . $values['User']['id'] . "'"; - $DB->doQuery($query); + $DB->update( + 'glpi_users', + ['password' => Auth::getPasswordHash(Sanitizer::unsanitize($values['User']['password']))], + ['id' => $values['User']['id']], + ); } } From 08e6a2485fc71bd7e64cea89a86dbc1d7643748e Mon Sep 17 00:00:00 2001 From: Stanislas Kita <7335054+stonebuzz@users.noreply.github.com> Date: Tue, 8 Sep 2026 10:57:40 +0200 Subject: [PATCH 2/2] Test: add PHPUnit infrastructure and CSV formula escaping test Backport the test shipped with PR #656, along with the PHPUnit setup the GLPI 10.0 line was missing: the CI workflow runs PHPUnit as soon as a phpunit.xml exists at the plugin root, using the GLPI core binary. --- Makefile | 1 + hook.php | 6 ++ inc/model.class.php | 4 +- phpunit.xml | 7 ++ tests/bootstrap.php | 48 +++++++++++++ .../ClientInjectionEscapeCsvFormulaTest.php | 69 +++++++++++++++++++ 6 files changed, 134 insertions(+), 1 deletion(-) create mode 100644 Makefile create mode 100644 phpunit.xml create mode 100644 tests/bootstrap.php create mode 100644 tests/unit/ClientInjectionEscapeCsvFormulaTest.php diff --git a/Makefile b/Makefile new file mode 100644 index 00000000..ef1bed5a --- /dev/null +++ b/Makefile @@ -0,0 +1 @@ +include ../../PluginsMakefile.mk diff --git a/hook.php b/hook.php index 3091dece..9b86479d 100644 --- a/hook.php +++ b/hook.php @@ -210,6 +210,12 @@ function plugin_datainjection_uninstall() } } + // Remove plugin rights from all profiles + $profileRight = new ProfileRight(); + foreach (PluginDatainjectionProfile::getAllRights() as $right) { + $profileRight->deleteByCriteria(['name' => $right['field']]); + } + if (is_dir(PLUGIN_DATAINJECTION_UPLOAD_DIR)) { Toolbox::deleteDir(PLUGIN_DATAINJECTION_UPLOAD_DIR); } diff --git a/inc/model.class.php b/inc/model.class.php index b36d37b1..cb2e375e 100644 --- a/inc/model.class.php +++ b/inc/model.class.php @@ -28,6 +28,8 @@ * ------------------------------------------------------------------------- */ +use Glpi\Toolbox\Sanitizer; + class PluginDatainjectionModel extends CommonDBTM { public static $rightname = "plugin_datainjection_model"; @@ -1317,7 +1319,7 @@ public function switchReadyToUse() $tmp = $this->fields; $tmp['step'] = self::READY_TO_USE_STEP; - $tmp = Toolbox::addslashes_deep($tmp); + $tmp = Sanitizer::dbEscapeRecursive($tmp); $this->update($tmp); } diff --git a/phpunit.xml b/phpunit.xml new file mode 100644 index 00000000..4f3562ad --- /dev/null +++ b/phpunit.xml @@ -0,0 +1,7 @@ + + + + tests + + + diff --git a/tests/bootstrap.php b/tests/bootstrap.php new file mode 100644 index 00000000..42cde921 --- /dev/null +++ b/tests/bootstrap.php @@ -0,0 +1,48 @@ +. + * ------------------------------------------------------------------------- + * @copyright Copyright (C) 2007-2023 by DataInjection plugin team. + * @license GPLv2 https://www.gnu.org/licenses/gpl-2.0.html + * @link https://github.com/pluginsGLPI/datainjection + * ------------------------------------------------------------------------- + */ + +global $CFG_GLPI, $PLUGIN_HOOKS; + +define('GLPI_ROOT', dirname(__DIR__, 3)); +define('GLPI_LOG_DIR', GLPI_ROOT . '/files/_logs'); +define('TU_USER', 'glpi'); +define('TU_PASS', 'glpi'); +define('GLPI_LOG_LVL', 'DEBUG'); + +require GLPI_ROOT . '/inc/includes.php'; + +include_once GLPI_ROOT . '/phpunit/GLPITestCase.php'; +include_once GLPI_ROOT . '/phpunit/DbTestCase.php'; + +require_once __DIR__ . '/../setup.php'; + +if (!Plugin::isPluginActive('datainjection')) { + throw new RuntimeException('Plugin datainjection is not active in the test database'); +} diff --git a/tests/unit/ClientInjectionEscapeCsvFormulaTest.php b/tests/unit/ClientInjectionEscapeCsvFormulaTest.php new file mode 100644 index 00000000..74d5c813 --- /dev/null +++ b/tests/unit/ClientInjectionEscapeCsvFormulaTest.php @@ -0,0 +1,69 @@ +. + * ------------------------------------------------------------------------- + * @copyright Copyright (C) 2007-2023 by DataInjection plugin team. + * @license GPLv2 https://www.gnu.org/licenses/gpl-2.0.html + * @link https://github.com/pluginsGLPI/datainjection + * ------------------------------------------------------------------------- + */ + +require_once dirname(__DIR__, 2) . '/inc/clientinjection.class.php'; + +/** + * Covers escapeCsvFormula(), which prefixes values starting with a CSV + * formula-injection trigger character with a single quote before they are + * written out by exportErrorsInCSV(). + */ +class ClientInjectionEscapeCsvFormulaTest extends DbTestCase +{ + public function escapeCsvFormulaProvider(): array + { + return [ + 'empty string' => ['', ''], + 'equals trigger' => ['=SUM(A1:A2)', "'=SUM(A1:A2)"], + 'plus trigger' => ['+1234', "'+1234"], + 'minus trigger' => ['-1234', "'-1234"], + 'at trigger' => ['@SUM(A1:A2)', "'@SUM(A1:A2)"], + 'safe value passthrough' => ['normal value', 'normal value'], + 'non-string passthrough' => [42, 42], + ]; + } + + /** + * @dataProvider escapeCsvFormulaProvider + */ + public function testEscapeCsvFormula($value, $expected): void + { + $escape_csv_formula = new ReflectionMethod( + PluginDatainjectionClientInjection::class, + 'escapeCsvFormula' + ); + // Required on PHP < 8.1, where private members stay closed to reflection + if (PHP_VERSION_ID < 80100) { + $escape_csv_formula->setAccessible(true); + } + + $this->assertSame($expected, $escape_csv_formula->invoke(null, $value)); + } +}