diff --git a/.gitignore b/.gitignore index 283db0af..083958c7 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,5 @@ dist/ vendor/ .gh_token *.min.* +tests/files/ +.phpunit.result.cache diff --git a/CHANGELOG.md b/CHANGELOG.md index 8095c79b..eee0a10c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] - Fix additional fields being saved on an item the user is not allowed to update. +- Fix additional fields being displayed for an item the user is not allowed to read - Fix invalid characters being kept in the generated field name. - Fix missing right checks on the target item when displaying or saving additional fields values diff --git a/ajax/container.php b/ajax/container.php index 31dbb324..a2e4de42 100644 --- a/ajax/container.php +++ b/ajax/container.php @@ -46,18 +46,28 @@ $items_id = (int) $_GET['items_id']; $type = $_GET['type']; $subtype = $_GET['subtype']; - $input = $_GET['input']; + $input = is_array($_GET['input'] ?? null) ? $_GET['input'] : []; - $item = new $itemtype(); - if ($items_id > 0) { - if (!$item->getFromDB($items_id)) { - Response::sendError(404, 'Not Found'); - } + if ($items_id > 0 && !PluginFieldsContainer::canReadTargetItem($itemtype, $items_id)) { + Response::sendError(403, 'Forbidden'); + return; + } + + $item = (new DbUtils())->getItemForItemtype($itemtype); + if (!$item instanceof CommonDBTM) { + Response::sendError(404, 'Not Found'); + return; + } + + if ($items_id > 0) { if (!$item->can($items_id, READ)) { Response::sendError(403, 'Forbidden'); return; } + } elseif (!$item->can(0, CREATE, $input)) { + Response::sendError(403, 'Forbidden'); + return; } $item->input = $input; diff --git a/inc/container.class.php b/inc/container.class.php index b7e0c9bc..b7540a86 100644 --- a/inc/container.class.php +++ b/inc/container.class.php @@ -1235,10 +1235,26 @@ public static function displayTabContentForItem(CommonGLPI $item, $tabnum = 1, $ * @param integer $items_id Item id */ public static function canUpdateTargetItem(string $itemtype, int $items_id): bool + { + return self::canTargetItem($itemtype, $items_id, UPDATE); + } + + /** + * Check that current user is allowed to read the item the fields values are attached to + * + * @param string $itemtype Item type + * @param integer $items_id Item id + */ + public static function canReadTargetItem(string $itemtype, int $items_id): bool + { + return self::canTargetItem($itemtype, $items_id, READ); + } + + private static function canTargetItem(string $itemtype, int $items_id, int $right): bool { $item = (new DbUtils())->getItemForItemtype($itemtype); - return $item instanceof CommonDBTM && $item->can($items_id, UPDATE); + return $item instanceof CommonDBTM && $item->can($items_id, $right); } /** diff --git a/phpunit.xml b/phpunit.xml new file mode 100644 index 00000000..5621cf58 --- /dev/null +++ b/phpunit.xml @@ -0,0 +1,17 @@ + + + + + inc + + + + + + tests + + + diff --git a/tests/FieldTestCase.php b/tests/FieldTestCase.php new file mode 100644 index 00000000..af81756c --- /dev/null +++ b/tests/FieldTestCase.php @@ -0,0 +1,102 @@ +. + * ------------------------------------------------------------------------- + * @copyright Copyright (C) 2013-2023 by Fields plugin team. + * @license GPLv2 https://www.gnu.org/licenses/gpl-2.0.html + * @link https://github.com/pluginsGLPI/fields + * ------------------------------------------------------------------------- + */ + +namespace GlpiPlugin\Field\Tests; + +use DBmysql; +use PluginFieldsContainer; +use PluginFieldsField; + +trait FieldTestTrait +{ + /** @var PluginFieldsContainer[] */ + private static array $createdContainers = []; + + /** @var PluginFieldsField[] */ + private static array $createdFields = []; + + public function tearDownFieldTest(): void + { + // Re-login to ensure we are logged in + $this->login(); + + array_map( + fn(PluginFieldsContainer $container) => $container->delete($container->fields, true), + self::$createdContainers, + ); + self::$createdContainers = []; + + array_map( + fn(PluginFieldsField $field) => $field->delete($field->fields, true), + self::$createdFields, + ); + self::$createdFields = []; + + /** @var DBmysql $DB */ + global $DB; + $DB->clearSchemaCache(); + } + + public function createFieldContainer(array $inputs): PluginFieldsContainer + { + // Re-login to ensure we are logged in + $this->login(); + + $container = $this->createItem(PluginFieldsContainer::class, $inputs, ['itemtypes']); + self::$createdContainers[] = $container; + + // Re-initialize fields plugin to register new container logic + plugin_init_fields(); + + /** @var DBmysql $DB */ + global $DB; + $DB->clearSchemaCache(); + + return $container; + } + + public function createField(array $inputs): PluginFieldsField + { + // Re-login to ensure we are logged in + $this->login(); + + $field = $this->createItem(PluginFieldsField::class, $inputs, ['allowed_values']); + self::$createdFields[] = $field; + + // Re-initialize fields plugin to register new field logic + plugin_init_fields(); + + /** @var DBmysql $DB */ + global $DB; + $DB->clearSchemaCache(); + + return $field; + } +} diff --git a/tests/Units/ContainerItemRightTest.php b/tests/Units/ContainerItemRightTest.php new file mode 100644 index 00000000..34bd7e1d --- /dev/null +++ b/tests/Units/ContainerItemRightTest.php @@ -0,0 +1,195 @@ +. + * ------------------------------------------------------------------------- + * @copyright Copyright (C) 2013-2023 by Fields plugin team. + * @license GPLv2 https://www.gnu.org/licenses/gpl-2.0.html + * @link https://github.com/pluginsGLPI/fields + * ------------------------------------------------------------------------- + */ + +declare(strict_types=1); + +namespace GlpiPlugin\Field\Tests\Units; + +use Computer; +use DbTestCase; +use Entity; +use GLPITestCase; +use GlpiPlugin\Field\Tests\FieldTestTrait; +use PluginFieldsContainer; +use PluginFieldsField; +use PluginFieldsProfile; + +require_once __DIR__ . '/../FieldTestCase.php'; + +final class ContainerItemRightTest extends DbTestCase +{ + use FieldTestTrait; + + public function setUp(): void + { + GLPITestCase::setUp(); + + global $CFG_GLPI; + $CFG_GLPI["event_loglevel"] = 0; + + $this->login(); + } + + public function tearDown(): void + { + $this->tearDownFieldTest(); + GLPITestCase::tearDown(); + } + + public function testCanUpdateTargetItemFollowsRightOnItem(): void + { + global $CFG_GLPI; + $CFG_GLPI["event_loglevel"] = 0; + + $this->login(); + $entity_id = getItemByTypeName(Entity::class, '_test_root_entity', true); + $this->setEntity($entity_id, true); + $computer = $this->createItem(Computer::class, [ + 'name' => 'Computer ' . $this->getUniqueString(), + 'entities_id' => $entity_id, + ]); + + $this->assertTrue(PluginFieldsContainer::canUpdateTargetItem(Computer::class, $computer->getID())); + + $this->login('post-only', 'postonly'); + $this->setEntity($entity_id, true); + + $this->assertFalse(PluginFieldsContainer::canUpdateTargetItem(Computer::class, $computer->getID())); + } + + public function testCanUpdateTargetItemRejectsInvalidItemtype(): void + { + global $CFG_GLPI; + $CFG_GLPI["event_loglevel"] = 0; + + $this->login(); + + $this->assertFalse(PluginFieldsContainer::canUpdateTargetItem('', 1)); + } + + public function testCanReadTargetItemFollowsRightOnItem(): void + { + global $CFG_GLPI; + $CFG_GLPI["event_loglevel"] = 0; + + $this->login(); + $root_entity_id = getItemByTypeName(Entity::class, '_test_root_entity', true); + $child_entity = $this->createItem(Entity::class, [ + 'name' => 'Entity ' . $this->getUniqueString(), + 'entities_id' => $root_entity_id, + ]); + $computer = $this->createItem(Computer::class, [ + 'name' => 'Computer ' . $this->getUniqueString(), + 'entities_id' => $child_entity->getID(), + ]); + + $this->assertTrue(PluginFieldsContainer::canReadTargetItem(Computer::class, $computer->getID())); + + $this->setEntity($root_entity_id, false); + + $this->assertFalse(PluginFieldsContainer::canReadTargetItem(Computer::class, $computer->getID())); + } + + public function testCanReadTargetItemRejectsUnknownItem(): void + { + global $CFG_GLPI; + $CFG_GLPI["event_loglevel"] = 0; + + $this->login(); + + $this->assertFalse(PluginFieldsContainer::canReadTargetItem('', 1)); + $this->assertFalse(PluginFieldsContainer::canReadTargetItem(Computer::class, 999999)); + } + + public function testShowDomContainerRendersReadOnlyFieldsWithoutUpdateRight(): void + { + $entity_id = getItemByTypeName(Entity::class, '_test_root_entity', true); + $this->setEntity($entity_id, true); + + $container = $this->createFieldContainer([ + // Digits are spelled out in the generated system name, keep the label short and digit-free + 'label' => 'Dom container', + 'type' => 'dom', + 'itemtypes' => [Computer::class], + 'is_active' => 1, + 'entities_id' => $entity_id, + 'is_recursive' => 1, + ]); + $field = $this->createField([ + 'label' => 'Dom field', + 'type' => 'text', + PluginFieldsContainer::getForeignKeyField() => $container->getID(), + 'ranking' => 1, + 'is_active' => 1, + 'is_readonly' => 0, + ]); + $computer = $this->createItem(Computer::class, [ + 'name' => 'Computer ' . $this->getUniqueString(), + 'entities_id' => $entity_id, + ]); + + $this->assertStringNotContainsString( + 'readonly', + $this->renderDomContainer($container->getID(), $computer), + ); + + $this->setRightOnContainer($container->getID(), READ); + + $this->assertStringContainsString( + 'readonly', + $this->renderDomContainer($container->getID(), $computer), + ); + + $this->setRightOnContainer($container->getID(), 0); + + $this->assertStringNotContainsString( + $field->fields['name'], + $this->renderDomContainer($container->getID(), $computer), + ); + } + + private function renderDomContainer(int $containers_id, Computer $computer): string + { + ob_start(); + PluginFieldsField::showDomContainer($containers_id, $computer); + + return (string) ob_get_clean(); + } + + private function setRightOnContainer(int $containers_id, int $right): void + { + $profile_right = new PluginFieldsProfile(); + $this->assertTrue($profile_right->getFromDBByCrit([ + 'profiles_id' => $_SESSION['glpiactiveprofile']['id'], + 'plugin_fields_containers_id' => $containers_id, + ])); + $this->updateItem(PluginFieldsProfile::class, $profile_right->getID(), ['right' => $right]); + } +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php new file mode 100644 index 00000000..5ebb18d7 --- /dev/null +++ b/tests/bootstrap.php @@ -0,0 +1,55 @@ +. + * ------------------------------------------------------------------------- + * @copyright Copyright (C) 2013-2023 by Fields plugin team. + * @license GPLv2 https://www.gnu.org/licenses/gpl-2.0.html + * @link https://github.com/pluginsGLPI/fields + * ------------------------------------------------------------------------- + */ + +/** @var array $CFG_GLPI */ +/** @var array $PLUGIN_HOOKS */ +global $CFG_GLPI, $PLUGIN_HOOKS; + +define('GLPI_ROOT', __DIR__ . '/../../../'); +define('GLPI_LOG_DIR', __DIR__ . '/files/_logs'); + +define('TU_USER', 'glpi'); +define('TU_PASS', 'glpi'); +define('GLPI_LOG_LVL', 'DEBUG'); + +require GLPI_ROOT . '/inc/includes.php'; + +Plugin::load('fields', true); + +// The `@fields` Twig namespace is only registered for plugins activated in DB, force it for tests +$twig_loader = Glpi\Application\View\TemplateRenderer::getInstance()->getEnvironment()->getLoader(); +if ($twig_loader instanceof Twig\Loader\FilesystemLoader) { + $twig_loader->addPath(__DIR__ . '/../templates', 'fields'); +} + +include_once GLPI_ROOT . '/phpunit/GLPITestCase.php'; +include_once GLPI_ROOT . '/phpunit/DbTestCase.php'; +require_once __DIR__ . '/../vendor/autoload.php'; +require_once __DIR__ . '/../setup.php';