diff --git a/CHANGELOG.md b/CHANGELOG.md index 30a305b2..cf2ff0cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed - 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 bbb7e7c7..f1030322 100644 --- a/ajax/container.php +++ b/ajax/container.php @@ -45,18 +45,25 @@ $items_id = (int) $_GET['items_id']; $type = $_GET['type']; $subtype = $_GET['subtype']; - $input = $_GET['input']; + $input = is_array($_GET['input'] ?? null) ? $_GET['input'] : []; + + if ($items_id > 0 && !PluginFieldsContainer::canReadTargetItem($itemtype, $items_id)) { + throw new AccessDeniedHttpException(); + } $dbu = new DbUtils(); $item = $dbu->getItemForItemtype($itemtype); - if ($items_id > 0) { - if (!$item->getFromDB($items_id)) { - throw new NotFoundHttpException(); - } + if (!$item instanceof CommonDBTM) { + throw new NotFoundHttpException(); + } + + if ($items_id > 0) { if (!$item->can($items_id, READ)) { throw new AccessDeniedHttpException(); } + } elseif (!$item->can(0, CREATE, $input)) { + throw new AccessDeniedHttpException(); } $item->input = $input; diff --git a/inc/container.class.php b/inc/container.class.php index 1376f344..805a9b50 100644 --- a/inc/container.class.php +++ b/inc/container.class.php @@ -1362,10 +1362,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/tests/Units/ContainerItemRightTest.php b/tests/Units/ContainerItemRightTest.php index 449c015d..ae31b33c 100644 --- a/tests/Units/ContainerItemRightTest.php +++ b/tests/Units/ContainerItemRightTest.php @@ -35,10 +35,30 @@ use Computer; use Entity; use Glpi\Tests\DbTestCase; +use Glpi\Tests\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(); + $this->login(); + } + + public function tearDown(): void + { + $this->tearDownFieldTest(); + GLPITestCase::tearDown(); + } + public function testCanUpdateTargetItemFollowsRightOnItem(): void { $this->login(); @@ -63,4 +83,96 @@ public function testCanUpdateTargetItemRejectsInvalidItemtype(): void $this->assertFalse(PluginFieldsContainer::canUpdateTargetItem('', 1)); } + + public function testCanReadTargetItemFollowsRightOnItem(): void + { + $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 + { + $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([ + 'label' => 'Dom container ' . $this->getUniqueString(), + '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]); + } }