Fix(Core): item validation when loading trees

.gitignore

@@ -1,4 +1,5 @@
 dist/
 vendor/
 .gh_token
-*.min.*
+*.min.*
+.phpunit.result.cache

CHANGELOG.md

@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
+## [Unreleased]
+
+### Fixed
+
+- Enforce item rights and sanitize inputs in tree loading
+
 ## [1.20.0] - 2025-09-16
 
 ### Added

composer.json

@@ -8,6 +8,11 @@
         "php-parallel-lint/php-parallel-lint": "^1.4",
         "phpstan/phpstan": "^2.1"
     },
+    "autoload": {
+        "psr-4": {
+            "GlpiPlugin\\Treeview\\Tests\\": "tests"
+        }
+    },
     "config": {
         "optimize-autoloader": true,
         "platform": {

front/config.form.php

@@ -30,10 +30,25 @@
 
 include('../../../inc/includes.php');
 
-Session::checkRight('treeview', UPDATE);
+Session::checkRight(PluginTreeviewConfig::$rightname, UPDATE);
 
 $config = new PluginTreeviewConfig();
 if (isset($_POST['update'])) {
+    if (isset($_POST['target']) && !in_array($_POST['target'], ['_blank', 'right'], true)) {
+        Session::addMessageAfterRedirect(
+            sprintf(__s('Invalid target value (%s) ignored', 'treeview'), htmlspecialchars((string) $_POST['target'], ENT_QUOTES, 'UTF-8')),
+            false,
+            ERROR,
+        );
+        unset($_POST['target']);
+    }
+
+    foreach (['folderLinks', 'useSelection', 'useLines', 'useIcons', 'closeSameLevel', 'itemName', 'locationName'] as $field) {
+        if (isset($_POST[$field])) {
+            $_POST[$field] = (int) $_POST[$field];
+        }
+    }
+
     $config->update($_POST);
     Html::back();
 } else {

front/preference.form.php

@@ -36,6 +36,14 @@
 
 //Save user preferences
 if (isset($_POST['plugin_treeview_user_preferences_save'])) {
-    $pref->update($_POST);
+    if (!($own_id = $pref->checkIfPreferenceExists(Session::getLoginUserID()))) {
+        $own_id = $pref->addDefaultPreference(Session::getLoginUserID());
+        if (!$own_id) {
+            Session::addMessageAfterRedirect(__s('Unable to save preferences', 'treeview'), false, ERROR);
+            Html::back();
+        }
+    }
+
+    $pref->update(['id' => $own_id, 'show_on_load' => (int) ($_POST['show_on_load'] ?? 0)]);
     Html::back();
 }

inc/config.class.php

@@ -51,6 +51,8 @@ class PluginTreeviewConfig extends CommonDBTM
         'PassiveDCEquipment',
     ];
 
+    public static $rightname = 'config';
+
     /**
     * Display name of itemtype
     *
@@ -306,7 +308,7 @@ public function getNodesFromDb()
         $closeSameLevel = $this->fields['closeSameLevel'];
 
         // Load the settings in JavaSript so that dTree script can apply them
-        echo "d.config.target         = '" . $target . "';\n";
+        echo "d.config.target         = " . json_encode($target) . ";\n";
         echo 'd.config.folderLinks    = ' . $folderLinks . ";\n";
         echo 'd.config.useSelection   = ' . $useSelection . ";\n";
         echo 'd.config.useLines       = ' . $useLines . ";\n";
@@ -333,7 +335,7 @@ public function getNodesFromDb()
         // Is this the first time we load the page?
         if (isset($_GET['nodes']) && $_GET['nodes'] != '') {
             // If no then get all the nodes requested by the client
-            $nodes = array_reverse(explode('.', $_GET['nodes']));
+            $nodes = array_map('intval', array_reverse(explode('.', $_GET['nodes'])));
         } else {
             // If yes then get only the root node
             $nodes[0] = 0;
@@ -389,7 +391,7 @@ public function getNodesFromDb()
                                 "\", true, -1,'');\n";
                         $dontLoad = 'true';
                         // Then add aloso its items
-                        foreach (self::$types as $type) {
+                        foreach (self::getTypes() as $type) {
                             $item      = new $type();
                             $itemtable = getTableForItemType($type);
 
@@ -408,7 +410,7 @@ public function getNodesFromDb()
                                 $criteria['WHERE']['is_deleted'] = 0;
                             }
 
-                            if ($this->isEntityAssign()) {
+                            if ($item->isEntityAssign()) {
                                 $criteria['WHERE']['entities_id'] = $_SESSION['glpiactive_entity'];
                             }
 
@@ -517,9 +519,9 @@ public function getNodesFromDb()
 
         // Open the tree to the desired node
         if ($openedType != -1) {
-            echo 'd.openTo(' . $openedType . ");\n";
+            echo 'd.openTo(' . (int) $openedType . ");\n";
         } else {
-            echo 'd.openTo(' . $nodes[count($nodes) - 1] . ");\n";
+            echo 'd.openTo(' . (int) $nodes[count($nodes) - 1] . ");\n";
         }
     }
 

inc/preference.class.php

@@ -65,8 +65,7 @@ public function showFormUserPreference($target, $id)
 
         echo "<tr class='tab_bg_1 center'><td colspan='2'>";
         echo "<input type='submit' name='plugin_treeview_user_preferences_save' value='" .
-             _sx('button', 'Post') . "' class='submit'>";
-        echo "<input type='hidden' name='id' value='$id'></td></tr>";
+             _sx('button', 'Post') . "' class='submit'></td></tr>";
 
         echo "<tr class='tab_bg_1 center'>";
         echo "<td colspan='2'>" . __('Warning: If there are more than one plugin which be loaded at startup, then only the first will be used', 'treeview');

index.php

@@ -30,6 +30,12 @@
 
 include('../../inc/includes.php');
 
+Session::checkLoginUser();
+
+if (empty($_SESSION['glpi_plugin_treeview_profile']['treeview'])) {
+    Html::displayRightError();
+}
+
 Plugin::load('treeview', true);
 
 $_SESSION['glpi_plugin_treeview_loaded'] = 1;

left.php

@@ -32,6 +32,10 @@
 
 Session::checkLoginUser();
 
+if (empty($_SESSION['glpi_plugin_treeview_profile']['treeview'])) {
+    Html::displayRightError();
+}
+
 $treeview_url = Plugin::getWebDir('treeview');
 
 Html::includeHeader('TreeView');

phpunit.xml

@@ -0,0 +1,7 @@
+<phpunit bootstrap="tests/bootstrap.php" colors="true" testdox="true">
+   <testsuites>
+      <testsuite name="Tests">
+         <directory>tests</directory>
+      </testsuite>
+   </testsuites>
+</phpunit>

tests/TreeviewTestCase.php

@@ -0,0 +1,69 @@
+<?php
+
+/**
+ * -------------------------------------------------------------------------
+ * TreeView plugin for GLPI
+ * -------------------------------------------------------------------------
+ *
+ * LICENSE
+ *
+ * This file is part of TreeView.
+ *
+ * TreeView 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 2 of the License, or
+ * (at your option) any later version.
+ *
+ * TreeView 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 TreeView. If not, see <http://www.gnu.org/licenses/>.
+ * -------------------------------------------------------------------------
+ * @copyright Copyright (C) 2007-2023 by Teclib'.
+ * @license   GPLv2 https://www.gnu.org/licenses/gpl-2.0.html
+ * @link      https://github.com/pluginsGLPI/treeview
+ * -------------------------------------------------------------------------
+ */
+
+namespace GlpiPlugin\Treeview\Tests;
+
+use DbTestCase;
+use PluginTreeviewConfig;
+
+abstract class TreeviewTestCase extends DbTestCase
+{
+    /**
+     * Capture the JavaScript tree markup generated by
+     * PluginTreeviewConfig::getNodesFromDb() for the current session
+     * (active entity, current user rights).
+     *
+     * @param int|null $opened_locations_id A top-level location id to expand,
+     *                                       so its items get loaded too (the
+     *                                       tree only loads items for nodes
+     *                                       present in the "nodes" path, same
+     *                                       as a client expanding a node).
+     */
+    protected function getTreeOutput(?int $opened_locations_id = null): string
+    {
+        $previous_nodes = $_GET['nodes'] ?? null;
+        if ($opened_locations_id !== null) {
+            $_GET['nodes'] = $opened_locations_id . '.0';
+        }
+
+        $config = new PluginTreeviewConfig();
+        ob_start();
+        $config->getNodesFromDb();
+        $output = ob_get_clean();
+
+        if ($previous_nodes === null) {
+            unset($_GET['nodes']);
+        } else {
+            $_GET['nodes'] = $previous_nodes;
+        }
+
+        return $output;
+    }
+}

tests/Units/ConfigTest.php

@@ -0,0 +1,120 @@
+<?php
+
+/**
+ * -------------------------------------------------------------------------
+ * TreeView plugin for GLPI
+ * -------------------------------------------------------------------------
+ *
+ * LICENSE
+ *
+ * This file is part of TreeView.
+ *
+ * TreeView 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 2 of the License, or
+ * (at your option) any later version.
+ *
+ * TreeView 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 TreeView. If not, see <http://www.gnu.org/licenses/>.
+ * -------------------------------------------------------------------------
+ * @copyright Copyright (C) 2007-2023 by Teclib'.
+ * @license   GPLv2 https://www.gnu.org/licenses/gpl-2.0.html
+ * @link      https://github.com/pluginsGLPI/treeview
+ * -------------------------------------------------------------------------
+ */
+
+namespace GlpiPlugin\Treeview\Tests\Units;
+
+use Computer;
+use Entity;
+use GlpiPlugin\Treeview\Tests\TreeviewTestCase;
+use Location;
+use Profile;
+use ProfileRight;
+
+final class ConfigTest extends TreeviewTestCase
+{
+    public function testGetNodesFromDbOnlyShowsActiveEntityData(): void
+    {
+        $this->login();
+        $root_id = $this->getTestRootEntity(true);
+
+        $entity_a = $this->createItem(Entity::class, [
+            'name'        => 'treeview_entity_a_' . $this->getUniqueString(),
+            'entities_id' => $root_id,
+        ]);
+        $entity_b = $this->createItem(Entity::class, [
+            'name'        => 'treeview_entity_b_' . $this->getUniqueString(),
+            'entities_id' => $root_id,
+        ]);
+
+        $location_a = $this->createItem(Location::class, [
+            'name'        => 'treeview_loc_a_' . $this->getUniqueString(),
+            'entities_id' => $entity_a->getID(),
+        ]);
+        $location_b = $this->createItem(Location::class, [
+            'name'        => 'treeview_loc_b_' . $this->getUniqueString(),
+            'entities_id' => $entity_b->getID(),
+        ]);
+
+        $computer_a = $this->createItem(Computer::class, [
+            'name'         => 'treeview_computer_a_' . $this->getUniqueString(),
+            'entities_id'  => $entity_a->getID(),
+            'locations_id' => $location_a->getID(),
+        ]);
+        $computer_b = $this->createItem(Computer::class, [
+            'name'         => 'treeview_computer_b_' . $this->getUniqueString(),
+            'entities_id'  => $entity_b->getID(),
+            'locations_id' => $location_b->getID(),
+        ]);
+
+        // Switch active entity A only (not recursive).
+        $this->setEntity($entity_a->getID(), false);
+
+        $output = $this->getTreeOutput($location_a->getID());
+
+        $this->assertStringContainsString($computer_a->fields['name'], $output);
+        $this->assertStringNotContainsString($computer_b->fields['name'], $output);
+        $this->assertStringNotContainsString($location_b->fields['name'], $output);
+    }
+
+    public function testGetNodesFromDbHidesItemtypeWithoutViewRight(): void
+    {
+        $this->login();
+        $entity_id = $this->getTestRootEntity(true);
+
+        $location = $this->createItem(Location::class, [
+            'name'        => 'treeview_loc_' . $this->getUniqueString(),
+            'entities_id' => $entity_id,
+        ]);
+        $computer = $this->createItem(Computer::class, [
+            'name'         => 'treeview_computer_' . $this->getUniqueString(),
+            'entities_id'  => $entity_id,
+            'locations_id' => $location->getID(),
+        ]);
+
+        $super_admin_id  = getItemByTypeName(Profile::class, 'Super-Admin', true);
+        $original_rights = ProfileRight::getProfileRights($super_admin_id, [Computer::$rightname]);
+
+        // remove all rights to view computers
+        ProfileRight::updateProfileRights($super_admin_id, [
+            Computer::$rightname => $original_rights[Computer::$rightname] & ~(READ),
+        ]);
+        try {
+            $this->login('glpi');
+            $this->setEntity($entity_id, false);
+            $output = $this->getTreeOutput($location->getID());
+
+            $this->assertStringNotContainsString($computer->fields['name'], $output);
+        } finally {
+            ProfileRight::updateProfileRights($super_admin_id, [
+                Computer::$rightname => $original_rights[Computer::$rightname],
+            ]);
+        }
+    }
+}