From f5b07bb389a5fb082779f74f204e9770d3662849 Mon Sep 17 00:00:00 2001 From: Stanislas Kita <7335054+stonebuzz@users.noreply.github.com> Date: Wed, 30 Sep 2026 18:08:52 +0200 Subject: [PATCH 1/3] Fix: apply missing checks on block and field handling --- CHANGELOG.md | 8 + front/container.form.php | 8 +- front/field.form.php | 6 +- inc/container.class.php | 186 ++++++++-- inc/field.class.php | 9 +- inc/inventory.class.php | 128 ------- setup.php | 12 +- src/Controller/QuestionTypeAjaxController.php | 18 +- .../Units/ContainerGlpiItemReferenceTest.php | 174 +++++++++ tests/Units/ContainerReadonlyValuesTest.php | 334 ++++++++++++++++++ tests/Units/ContainerTest.php | 42 +++ tests/Units/ExportBlockAsYamlTest.php | 125 +++++++ .../Units/QuestionTypeAjaxControllerTest.php | 90 +++++ 13 files changed, 972 insertions(+), 168 deletions(-) delete mode 100644 inc/inventory.class.php create mode 100644 tests/Units/ContainerGlpiItemReferenceTest.php create mode 100644 tests/Units/ContainerReadonlyValuesTest.php create mode 100644 tests/Units/ExportBlockAsYamlTest.php create mode 100644 tests/Units/QuestionTypeAjaxControllerTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ecf7d67..189e600a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Fix a field's default value not being applied to existing items and not being shown in search results for items with no dedicated row in the container table - Fix mandatory fields blocking automated item creation - Fix unclear mandatory field error when a GLPI form creating a ticket does not provide the field. +- Fix blocks export, block deletion, read-only fields, item fields and form editor field selection not applying the expected checks. +- Fix read-only status overrides being resolved from the previous status instead of the submitted one. +- Fix read-only fields of an "Insertion in form" block being overwritable from the item form. +- Remove obsolete FusionInventory inventory hook. + +### Changed + +- A "GLPI item" field now rejects a new reference to an item the current user cannot read, instead of silently clearing it. Automated writes (CLI, cron, inventory) are not affected. ## [1.24.5] - 2026-09-11 diff --git a/front/container.form.php b/front/container.form.php index 6b5dafa1..c7440773 100644 --- a/front/container.form.php +++ b/front/container.form.php @@ -46,9 +46,9 @@ $container->check($_POST['id'], DELETE); $ok = $container->delete($_POST); Html::redirect(PLUGINFIELDS_WEB_DIR . '/front/container.php'); -} elseif (isset($_REQUEST['purge'])) { - $container->check($_REQUEST['id'], PURGE); - $container->delete($_REQUEST, true); +} elseif (isset($_POST['purge'])) { + $container->check($_POST['id'], PURGE); + $container->delete($_POST, true); Html::redirect(PLUGINFIELDS_WEB_DIR . '/front/container.php'); } elseif (isset($_POST['update'])) { $container->check($_POST['id'], UPDATE); @@ -67,7 +67,7 @@ throw new AccessDeniedHttpException(); } - $container->updateFieldsValues($_REQUEST, $_REQUEST['itemtype'], false); + $container->updateFieldsValues(PluginFieldsContainer::removeReadonlyValues($_REQUEST, $item), $_REQUEST['itemtype'], false); } Html::back(); diff --git a/front/field.form.php b/front/field.form.php index 01c4d1b5..cb888e6a 100644 --- a/front/field.form.php +++ b/front/field.form.php @@ -45,9 +45,9 @@ $field->check($_POST['id'], DELETE); $field->delete($_POST); Html::back(); -} elseif (isset($_REQUEST['purge'])) { - $field->check($_REQUEST['id'], PURGE); - $field->delete($_REQUEST, true); +} elseif (isset($_POST['purge'])) { + $field->check($_POST['id'], PURGE); + $field->delete($_POST, true); $field->redirectToList(); } elseif (isset($_POST['update'])) { $field->check($_POST['id'], UPDATE); diff --git a/inc/container.class.php b/inc/container.class.php index 8221cd1e..d77d1137 100644 --- a/inc/container.class.php +++ b/inc/container.class.php @@ -257,12 +257,12 @@ public static function installUserData(Migration $migration, $version) foreach ($itemtypes as $itemtype) { $sysname = self::getSystemName($itemtype, $container['name']); - $class_filename = $sysname . '.class.php'; + $class_filename = basename($sysname) . '.class.php'; if (file_exists(PLUGINFIELDS_DIR . ('/inc/' . $class_filename))) { unlink(PLUGINFIELDS_DIR . ('/inc/' . $class_filename)); } - $injclass_filename = $sysname . 'injection.class.php'; + $injclass_filename = basename($sysname) . 'injection.class.php'; if (file_exists(PLUGINFIELDS_DIR . ('/inc/' . $injclass_filename))) { unlink(PLUGINFIELDS_DIR . ('/inc/' . $injclass_filename)); } @@ -646,6 +646,14 @@ public function prepareInputForAdd($input) $input['itemtypes'] = [$input['itemtypes']]; } + foreach ($input['itemtypes'] as $itemtype) { + if (!is_string($itemtype) || preg_match('/^[A-Za-z_][A-Za-z0-9_\\\\]*$/', $itemtype) !== 1) { + Session::AddMessageAfterRedirect(__('At least one selected object is not a valid element type', 'fields'), false, ERROR); + + return false; + } + } + if ($input['type'] === 'dom') { //check for already exist dom container with this itemtype $found = $this->find(['type' => 'dom']); @@ -774,8 +782,8 @@ public static function generateTemplate($fields) foreach ($itemtypes as $itemtype) { $sysname = self::getSystemName($itemtype, $fields['name']); $classname = self::getClassname($itemtype, $fields['name']); - $class_filename = $sysname . '.class.php'; - $injection_filename = $sysname . 'injection.class.php'; + $class_filename = basename($sysname) . '.class.php'; + $injection_filename = basename($sysname) . 'injection.class.php'; // prevent usage of plugin class if not loaded if (!class_exists($itemtype)) { @@ -854,8 +862,8 @@ public function pre_deleteItem() foreach (PluginFieldsToolbox::decodeJSONItemtypes($this->fields['itemtypes']) as $itemtype) { $classname = self::getClassname($itemtype, $this->fields['name']); $sysname = self::getSystemName($itemtype, $this->fields['name']); - $class_filename = $sysname . '.class.php'; - $injection_filename = $sysname . 'injection.class.php'; + $class_filename = basename($sysname) . '.class.php'; + $injection_filename = basename($sysname) . 'injection.class.php'; //delete fields $field_obj = new PluginFieldsField(); @@ -1683,6 +1691,7 @@ public static function validateValues($data, $itemtype, $massiveaction, $is_crea $empty_errors = []; $number_errors = []; $url_errors = []; + $reference_errors = []; $container = new self(); $container->getFromDB($data['plugin_fields_containers_id']); @@ -1714,6 +1723,8 @@ public static function validateValues($data, $itemtype, $massiveaction, $is_crea } } + $stored_values = self::getStoredValues($container, $itemtype, (int) ($data['items_id'] ?? 0)); + foreach ($fields as $field) { if (!$field['is_active']) { continue; @@ -1730,13 +1741,19 @@ public static function validateValues($data, $itemtype, $massiveaction, $is_crea $itemtype_key = sprintf('itemtype_%s', $name); $items_id_key = sprintf('items_id_%s', $name); - if ( - isset($data[$itemtype_key], $data[$items_id_key]) - && is_a($data[$itemtype_key], CommonDBTM::class, true) - && $data[$items_id_key] > 0 - ) { - $glpi_item = new $data[$itemtype_key](); - $value = $glpi_item->getFromDB($data[$items_id_key]) ? $data[$items_id_key] : null; + $is_unchanged_reference = isset($stored_values[$itemtype_key], $stored_values[$items_id_key], $data[$itemtype_key], $data[$items_id_key]) + && $stored_values[$itemtype_key] === $data[$itemtype_key] + && (int) $stored_values[$items_id_key] === (int) $data[$items_id_key]; + if (!$is_unchanged_reference && isset($data[$items_id_key]) && (int) $data[$items_id_key] > 0) { + $value = self::isValidItemReference($field, $data[$itemtype_key] ?? null, (int) $data[$items_id_key]) + ? (int) $data[$items_id_key] + : null; + if ($value === null) { + $field['itemtype'] = PluginFieldsField::getType(); + $reference_errors[] = PluginFieldsLabelTranslation::getLabelFor($field); + $valid = false; + continue; + } } } elseif (isset($data[$name])) { $value = $data[$name]; @@ -1796,9 +1813,135 @@ public static function validateValues($data, $itemtype, $massiveaction, $is_crea . ' : ' . implode(', ', $url_errors), false, ERROR); } + if ($reference_errors !== []) { + Session::AddMessageAfterRedirect(__('Some item fields reference an invalid item', 'fields') + . ' : ' . implode(', ', $reference_errors), false, ERROR); + } + return $valid; } + /** + * Status of the item for this request: the submitted one when present, otherwise the persisted one. + */ + private static function getStatusValue(CommonDBTM $item): ?int + { + $status_field_name = PluginFieldsStatusOverride::getStatusFieldName($item->getType()); + foreach ([$item->input, $item->fields] as $source) { + if (array_key_exists($status_field_name, $source) && $source[$status_field_name] !== '') { + return (int) $source[$status_field_name]; + } + } + + return null; + } + + public static function removeReadonlyValues(array $data, CommonDBTM $item): array + { + $container_id = (int) $data['plugin_fields_containers_id']; + $fields = (new PluginFieldsField())->find([ + 'plugin_fields_containers_id' => $container_id, + 'is_active' => 1, + ]); + + $status_value = self::getStatusValue($item); + $status_overrides = $status_value !== null + ? PluginFieldsStatusOverride::getOverridesForItemtypeAndStatus($container_id, $item->getType(), $status_value) + : []; + foreach ($status_overrides as $status_override) { + if (isset($fields[$status_override['plugin_fields_fields_id']])) { + $fields[$status_override['plugin_fields_fields_id']]['is_readonly'] = $status_override['is_readonly']; + } + } + + $container = new self(); + $container->getFromDB($container_id); + + $stored_values = self::getStoredValues($container, $item->getType(), (int) $item->getID()); + + foreach ($fields as $field) { + if (!$field['is_readonly']) { + continue; + } + + $dropdown_key = sprintf('plugin_fields_%sdropdowns_id', $field['name']); + unset($data[sprintf('_%s_defined', $field['name'])], $data[sprintf('_%s_defined', $dropdown_key)]); + foreach ([ + $field['name'], + $dropdown_key, + sprintf('itemtype_%s', $field['name']), + sprintf('items_id_%s', $field['name']), + ] as $input_key) { + unset($data[$input_key]); + if (array_key_exists($input_key, $stored_values)) { + $data[$input_key] = $field['multiple'] + ? json_decode((string) $stored_values[$input_key], true) + : $stored_values[$input_key]; + } + } + + if ($item->isNewItem()) { + $data += self::getDefaultInput($field); + } + } + + return $data; + } + + private static function getDefaultInput(array $field): array + { + $default = PluginFieldsField::getDefaultValue($field); + if ($default === null) { + return []; + } + + $input_key = $field['type'] === 'dropdown' + ? sprintf('plugin_fields_%sdropdowns_id', $field['name']) + : $field['name']; + + if (!$field['multiple']) { + return [$input_key => $default]; + } + + $decoded = json_decode((string) $default, true); + + return is_array($decoded) && $decoded !== [] ? [$input_key => $decoded] : []; + } + + private static function getStoredValues(self $container, string $itemtype, int $items_id): array + { + if ($items_id <= 0 || $container->isNewItem()) { + return []; + } + + $values_obj = (new DbUtils())->getItemForItemtype(self::getClassname($itemtype, $container->fields['name'])); + if ($values_obj === false || !$values_obj->getFromDBByCrit(['items_id' => $items_id])) { + return []; + } + + return $values_obj->fields; + } + + private static function isValidItemReference(array $field, mixed $itemtype, int $items_id): bool + { + $allowed_itemtypes = json_decode((string) $field['allowed_values'], true); + if ( + !is_string($itemtype) + || !is_a($itemtype, CommonDBTM::class, true) + || !is_array($allowed_itemtypes) + || !in_array($itemtype, $allowed_itemtypes, true) + ) { + return false; + } + + $item = new $itemtype(); + + // Automated writes (CLI, cron, inventory) run without a user whose rights could be checked + return Session::getLoginUserID() === false || Session::isInventory() + ? $item->getFromDB($items_id) + : $item->can($items_id, READ); + } + public static function findContainer($itemtype, $type = 'tab', $subtype = '') { $condition = [ @@ -2031,6 +2174,9 @@ private static function checkContainerMandatory(CommonDBTM $item, PluginFieldsCo $c_id = $loc_c->getID(); if (false !== ($data = self::populateData($c_id, $item))) { + // read-only fields keep their stored value, or their default on creation + $data = self::removeReadonlyValues($data, $item); + if (self::validateValues($data, $item::getType(), isset($_REQUEST['massiveaction'])) === false) { return false; } @@ -2054,10 +2200,8 @@ private static function checkContainerMandatory(CommonDBTM $item, PluginFieldsCo $data['is_dynamic'] = true; } - if (array_key_exists($status_field_name, $item->input) && $item->input[$status_field_name] !== '') { - $data[$status_field_name] = (int) $item->input[$status_field_name]; - } elseif (array_key_exists($status_field_name, $item->fields) && $item->fields[$status_field_name] !== '') { - $data[$status_field_name] = (int) $item->fields[$status_field_name]; + if (($status_value = self::getStatusValue($item)) !== null) { + $data[$status_field_name] = $status_value; } if (!$item->isNewItem()) { @@ -2131,13 +2275,7 @@ private static function populateData($c_id, CommonDBTM $item) } // Add status so it can be used with status overrides - $status_field_name = PluginFieldsStatusOverride::getStatusFieldName($item->getType()); - $data[$status_field_name] = null; - if (array_key_exists($status_field_name, $item->input) && $item->input[$status_field_name] !== '') { - $data[$status_field_name] = (int) $item->input[$status_field_name]; - } elseif (array_key_exists($status_field_name, $item->fields) && $item->fields[$status_field_name] !== '') { - $data[$status_field_name] = (int) $item->fields[$status_field_name]; - } + $data[PluginFieldsStatusOverride::getStatusFieldName($item->getType())] = self::getStatusValue($item); $has_fields = false; foreach ($fields as $field) { diff --git a/inc/field.class.php b/inc/field.class.php index 9a89e0d3..c7ab2bce 100644 --- a/inc/field.class.php +++ b/inc/field.class.php @@ -1052,6 +1052,9 @@ public static function showForTab($params) //JS to trigger any change and check if container need to be display or not $ajax_url = $CFG_GLPI['root_doc'] . '/plugins/fields/ajax/container.php'; $items_id = $item->isNewItem() ? 0 : $item->getID(); + $js_itemtype = json_encode($item::getType(), JSON_HEX_TAG | JSON_HEX_APOS | JSON_HEX_QUOT | JSON_HEX_AMP); + $js_type = json_encode($type, JSON_HEX_TAG | JSON_HEX_APOS | JSON_HEX_QUOT | JSON_HEX_AMP); + $js_subtype = json_encode($subtype, JSON_HEX_TAG | JSON_HEX_APOS | JSON_HEX_QUOT | JSON_HEX_AMP); echo Html::scriptBlock( <<. - * ------------------------------------------------------------------------- - * @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 - * ------------------------------------------------------------------------- - */ - -class PluginFieldsInventory extends CommonDBTM -{ - public static function updateInventory($params = []) - { - if ( - !empty($params) - && isset($params['inventory_data']) && !empty($params['inventory_data']) - ) { - $availaibleItemType = ['Computer', 'Printer', 'NetworkEquipment']; - foreach (array_keys($params['inventory_data']) as $itemtype) { - if (in_array($itemtype, $availaibleItemType, true)) { - $items_id = 0; - //retrieve items id switch itemtype - switch ($itemtype) { - case Computer::getType(): - $items_id = $params['computers_id']; - break; - - case NetworkEquipment::getType(): - $items_id = $params['networkequipments_id']; - break; - - case Printer::getType(): - $items_id = $params['printers_id']; - break; - } - - if (class_exists('PluginFusioninventoryInventoryComputerComputer')) { - if ($itemtype == Computer::getType()) { - //load inventory from DB because - //FI not update XML file if computer is not update - $db_info = new PluginFusioninventoryInventoryComputerComputer(); - if ($db_info->getFromDBByCrit(['computers_id' => $items_id])) { - $arrayinventory = unserialize(gzuncompress($db_info->fields['serialized_inventory'])); - if (isset($arrayinventory['custom'])) { - self::updateFields($arrayinventory['custom']['container'], $itemtype, $items_id); - } - } - } else { - //Load XML file because FI always update XML file and don't store inventory into DB - $file = self::loadXMLFile($itemtype, $items_id); - if ( - $file !== false - && class_exists('PluginFusioninventoryFormatconvert') - ) { - $arrayinventory = PluginFusioninventoryFormatconvert::XMLtoArray($file); - if (isset($arrayinventory['CUSTOM'])) { - self::updateFields($arrayinventory['CUSTOM']['CONTAINER'], $itemtype, $items_id); - } - } - } - } - } - } - } - } - - public static function updateFields($containersData, $itemtype, $items_id) - { - if (isset($containersData['ID'])) { - // $containersData contains only one element, encapsulate it into an array - $containersData = [$containersData]; - } - - foreach ($containersData as $key => $containerData) { - $container = new PluginFieldsContainer(); - $container->getFromDB($containerData['ID']); - $data = []; - $data['items_id'] = $items_id; - $data['itemtype'] = $itemtype; - $data['plugin_fields_containers_id'] = $containerData['ID']; - foreach ($containerData['FIELDS'] as $key => $value) { - $data[strtolower((string) $key)] = $value; - } - - $container->updateFieldsValues($data, $itemtype, false); - } - } - - public static function loadXMLFile($itemtype, $items_id) - { - $folder = substr((string) $items_id, 0, -1); - if ($folder === '' || $folder === '0') { - $folder = '0'; - } - - //Check if the file exists with the .xml extension (new format) - /** @phpstan-ignore-next-line */ - $file = PLUGIN_FUSIONINVENTORY_XML_DIR . strtolower((string) $itemtype) . '/' . $folder . '/' . $items_id; - if (file_exists($file . '.xml')) { - $file .= '.xml'; - } elseif (!file_exists($file)) { - return false; - } - - return simplexml_load_file($file, 'SimpleXMLElement', LIBXML_NOCDATA); - } -} diff --git a/setup.php b/setup.php index 7f668c71..79337df6 100644 --- a/setup.php +++ b/setup.php @@ -154,11 +154,6 @@ function plugin_init_fields() } } - if (Plugin::isPluginActive('fusioninventory')) { - $PLUGIN_HOOKS['fusioninventory_inventory']['fields'] - = ['PluginFieldsInventory', 'updateInventory']; - } - // complete rule engine $PLUGIN_HOOKS['use_rules']['fields'] = ['PluginFusioninventoryTaskpostactionRule']; $PLUGIN_HOOKS['rule_matched']['fields'] = 'plugin_fields_rule_matched'; @@ -330,6 +325,13 @@ function plugin_fields_exportBlockAsYaml($container_id = null) $container_obj = new PluginFieldsContainer(); $containers = $container_obj->find($where); foreach ($containers as $container) { + if ( + !Session::haveAccessToEntity((int) $container['entities_id'], (bool) $container['is_recursive']) + || PluginFieldsProfile::getRightOnContainer((int) ($_SESSION['glpiactiveprofile']['id'] ?? 0), (int) $container['id']) < READ + ) { + continue; + } + $itemtypes = ((string) $container['itemtypes'] !== '') ? PluginFieldsToolbox::decodeJSONItemtypes($container['itemtypes'], true) : []; diff --git a/src/Controller/QuestionTypeAjaxController.php b/src/Controller/QuestionTypeAjaxController.php index 0fc4a9b3..8a73e43f 100644 --- a/src/Controller/QuestionTypeAjaxController.php +++ b/src/Controller/QuestionTypeAjaxController.php @@ -31,10 +31,13 @@ namespace GlpiPlugin\Fields\Controller; use Glpi\Controller\AbstractController; +use Glpi\Exception\Http\AccessDeniedHttpException; +use Glpi\Exception\Http\NotFoundHttpException; use Glpi\Form\Form; use PluginFieldsContainer; use PluginFieldsField; use PluginFieldsQuestionType; +use Session; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Attribute\Route; @@ -57,6 +60,19 @@ public function __invoke(Request $request): Response return new Response('Invalid block_id', Response::HTTP_BAD_REQUEST); } + if (!Form::canUpdate()) { + throw new AccessDeniedHttpException(); + } + + $block = PluginFieldsContainer::getById((int) $block_id); + if ($block === false) { + throw new NotFoundHttpException(); + } + + if (!Session::haveAccessToEntity($block->fields['entities_id'], (bool) $block->fields['is_recursive'])) { + throw new AccessDeniedHttpException(); + } + // Get available fields for the selected block $available_fields = PluginFieldsQuestionType::getFieldsFromBlock((int) $block_id); @@ -73,7 +89,7 @@ public function __invoke(Request $request): Response } // Get the container and field details - $current_container = PluginFieldsContainer::getById((int) $block_id); + $current_container = $block; $current_field = PluginFieldsField::getById($current_field_id); if (!$current_container || !$current_field || empty($current_field->fields)) { diff --git a/tests/Units/ContainerGlpiItemReferenceTest.php b/tests/Units/ContainerGlpiItemReferenceTest.php new file mode 100644 index 00000000..85d7857f --- /dev/null +++ b/tests/Units/ContainerGlpiItemReferenceTest.php @@ -0,0 +1,174 @@ +. + * ------------------------------------------------------------------------- + * @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 Entity; +use Glpi\Tests\DbTestCase; +use Glpi\Tests\GLPITestCase; +use GlpiPlugin\Field\Tests\FieldTestTrait; +use Location; +use PluginFieldsContainer; + +require_once __DIR__ . '/../FieldTestCase.php'; + +final class ContainerGlpiItemReferenceTest extends DbTestCase +{ + use FieldTestTrait; + + private PluginFieldsContainer $container; + + private string $field_name; + + private Computer $holder; + + private Computer $root_target; + + private Computer $child_target; + + private int $root_entity_id; + + public function setUp(): void + { + GLPITestCase::setUp(); + $this->login(); + + $this->root_entity_id = getItemByTypeName(Entity::class, '_test_root_entity', true); + $child_entity_id = getItemByTypeName(Entity::class, '_test_child_1', true); + $this->setEntity($this->root_entity_id, true); + + $this->container = $this->createFieldContainer([ + 'label' => 'Item reference ' . $this->getUniqueString(), + 'type' => 'tab', + 'itemtypes' => [Computer::class], + 'is_active' => 1, + 'entities_id' => $this->root_entity_id, + 'is_recursive' => 1, + ]); + $field = $this->createField([ + 'label' => 'Linked computer', + 'type' => 'glpi_item', + PluginFieldsContainer::getForeignKeyField() => $this->container->getID(), + 'ranking' => 1, + 'is_active' => 1, + 'is_readonly' => 0, + 'allowed_values' => [Computer::class], + ]); + $this->field_name = $field->fields['name']; + + $this->holder = $this->createComputer($this->root_entity_id); + $this->root_target = $this->createComputer($this->root_entity_id); + $this->child_target = $this->createComputer($child_entity_id); + } + + public function tearDown(): void + { + $this->tearDownFieldTest(); + GLPITestCase::tearDown(); + } + + public function testValidReferenceIsAccepted(): void + { + $this->assertTrue($this->validate(Computer::class, $this->root_target->getID())); + } + + public function testUnknownItemIsRejected(): void + { + $this->assertFalse($this->validate(Computer::class, 999999)); + $this->assertReferenceError(); + } + + public function testItemtypeOutsideAllowedValuesIsRejected(): void + { + $location = $this->createItem(Location::class, [ + 'name' => 'Location ' . $this->getUniqueString(), + 'entities_id' => $this->root_entity_id, + ]); + + $this->assertFalse($this->validate(Location::class, $location->getID())); + $this->assertReferenceError(); + } + + public function testReferenceToItemWithoutReadRightIsRejected(): void + { + $this->setEntity($this->root_entity_id, false); + + $this->assertFalse($this->validate(Computer::class, $this->child_target->getID())); + $this->assertReferenceError(); + } + + public function testUnchangedReferenceStaysAcceptedWithoutReadRight(): void + { + $this->assertTrue($this->container->updateFieldsValues( + $this->buildData(Computer::class, $this->child_target->getID()), + Computer::class, + false, + )); + + $this->setEntity($this->root_entity_id, false); + + $this->assertTrue($this->validate(Computer::class, $this->child_target->getID())); + } + + private function validate(string $itemtype, int $items_id): bool + { + return PluginFieldsContainer::validateValues($this->buildData($itemtype, $items_id), Computer::class, false); + } + + private function buildData(string $itemtype, int $items_id): array + { + return [ + 'plugin_fields_containers_id' => $this->container->getID(), + 'itemtype' => Computer::class, + 'items_id' => $this->holder->getID(), + 'itemtype_' . $this->field_name => $itemtype, + 'items_id_' . $this->field_name => $items_id, + ]; + } + + private function assertReferenceError(): void + { + $this->hasSessionMessages(ERROR, ['Some item fields reference an invalid item : Linked computer']); + } + + private function createComputer(int $entities_id): Computer + { + $computer = $this->createItem(Computer::class, [ + 'name' => 'Computer ' . $this->getUniqueString(), + 'entities_id' => $entities_id, + ]); + $this->assertInstanceOf(Computer::class, $computer); + + return $computer; + } +} diff --git a/tests/Units/ContainerReadonlyValuesTest.php b/tests/Units/ContainerReadonlyValuesTest.php new file mode 100644 index 00000000..cfe1e6fa --- /dev/null +++ b/tests/Units/ContainerReadonlyValuesTest.php @@ -0,0 +1,334 @@ +. + * ------------------------------------------------------------------------- + * @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 Entity; +use Glpi\Tests\DbTestCase; +use Glpi\Tests\GLPITestCase; +use GlpiPlugin\Field\Tests\FieldTestTrait; +use PluginFieldsContainer; +use PluginFieldsDropdown; +use PluginFieldsField; +use PluginFieldsStatusOverride; +use State; + +require_once __DIR__ . '/../FieldTestCase.php'; + +final class ContainerReadonlyValuesTest extends DbTestCase +{ + use FieldTestTrait; + + private int $entity_id; + + public function setUp(): void + { + GLPITestCase::setUp(); + $this->login(); + $this->entity_id = getItemByTypeName(Entity::class, '_test_root_entity', true); + $this->setEntity($this->entity_id, true); + } + + public function tearDown(): void + { + $this->tearDownFieldTest(); + GLPITestCase::tearDown(); + } + + public function testReadonlyValueIsReplacedByStoredValue(): void + { + $container = $this->createContainer('tab'); + $readonly_name = $this->createTextField($container, 'Locked', 1); + $editable_name = $this->createTextField($container, 'Editable', 0); + $computer = $this->createComputer(); + + $this->storeValues($container, $computer, [$readonly_name => 'stored']); + + $data = PluginFieldsContainer::removeReadonlyValues( + $this->buildData($container, $computer, [$readonly_name => 'hacked', $editable_name => 'new']), + $computer, + ); + + $this->assertSame('stored', $data[$readonly_name]); + $this->assertSame('new', $data[$editable_name]); + } + + public function testReadonlyValueWithoutStoredRowIsDropped(): void + { + $container = $this->createContainer('tab'); + $readonly_name = $this->createTextField($container, 'Locked', 1); + $editable_name = $this->createTextField($container, 'Editable', 0); + $computer = $this->createComputer(); + + $data = PluginFieldsContainer::removeReadonlyValues( + $this->buildData($container, $computer, [ + $readonly_name => 'hacked', + '_' . $readonly_name . '_defined' => 1, + $editable_name => 'new', + ]), + $computer, + ); + + $this->assertArrayNotHasKey($readonly_name, $data); + $this->assertArrayNotHasKey('_' . $readonly_name . '_defined', $data); + $this->assertSame('new', $data[$editable_name]); + } + + public function testStatusOverrideTogglesReadonly(): void + { + $container = $this->createContainer('dom'); + $field_name = $this->createTextField($container, 'Locked when stocked', 0); + $state = $this->createItem(State::class, ['name' => 'State ' . $this->getUniqueString(), 'entities_id' => $this->entity_id]); + $computer = $this->createComputer(); + + $this->createItem(PluginFieldsStatusOverride::class, [ + PluginFieldsContainer::getForeignKeyField() => $container->getID(), + 'plugin_fields_fields_id' => $this->fieldIdByName($field_name), + 'itemtype' => Computer::class, + 'states' => [$state->getID()], + 'is_readonly' => 1, + 'mandatory' => 0, + ], ['states', PluginFieldsContainer::getForeignKeyField()]); + $this->storeValues($container, $computer, [$field_name => 'stored']); + + $data = PluginFieldsContainer::removeReadonlyValues( + $this->buildData($container, $computer, [$field_name => 'submitted']), + $computer, + ); + $this->assertSame('submitted', $data[$field_name]); + + $computer = $this->updateItem(Computer::class, $computer->getID(), ['states_id' => $state->getID()]); + + $data = PluginFieldsContainer::removeReadonlyValues( + $this->buildData($container, $computer, [$field_name => 'submitted']), + $computer, + ); + $this->assertSame('stored', $data[$field_name]); + } + + public function testStatusOverrideUsesSubmittedStatusOnUpdate(): void + { + $container = $this->createContainer('dom'); + $field_name = $this->createTextField($container, 'Locked when stocked', 0); + $state = $this->createItem(State::class, ['name' => 'State ' . $this->getUniqueString(), 'entities_id' => $this->entity_id]); + $computer = $this->createComputer(); + $this->createReadonlyOverride($container, $field_name, $state); + $this->storeValues($container, $computer, [$field_name => 'stored']); + + $computer->input = ['states_id' => $state->getID()]; + $data = PluginFieldsContainer::removeReadonlyValues( + $this->buildData($container, $computer, [$field_name => 'submitted']), + $computer, + ); + $this->assertSame('stored', $data[$field_name]); + + $computer = $this->updateItem(Computer::class, $computer->getID(), ['states_id' => $state->getID()]); + $computer->input = ['states_id' => 0]; + + $data = PluginFieldsContainer::removeReadonlyValues( + $this->buildData($container, $computer, [$field_name => 'submitted']), + $computer, + ); + $this->assertSame('submitted', $data[$field_name]); + } + + public function testStatusOverrideAppliesOnItemCreation(): void + { + $container = $this->createContainer('dom'); + $field_name = $this->createTextField($container, 'Locked when stocked', 0, 'default'); + $state = $this->createItem(State::class, ['name' => 'State ' . $this->getUniqueString(), 'entities_id' => $this->entity_id]); + $this->createReadonlyOverride($container, $field_name, $state); + + $computer = new Computer(); + $computer->input = ['states_id' => $state->getID()]; + + $data = PluginFieldsContainer::removeReadonlyValues( + [$field_name => 'hacked', 'plugin_fields_containers_id' => $container->getID(), 'itemtype' => Computer::class, 'items_id' => 0], + $computer, + ); + $this->assertSame('default', $data[$field_name]); + } + + public function testReadonlyMultipleDropdownIsRestoredAsArray(): void + { + $container = $this->createContainer('tab'); + $field = $this->createField([ + 'label' => 'Tags', + 'type' => 'dropdown', + 'multiple' => 1, + 'default_value' => [], + PluginFieldsContainer::getForeignKeyField() => $container->getID(), + 'ranking' => 1, + 'is_active' => 1, + 'is_readonly' => 1, + ], ['default_value']); + $dropdown_key = sprintf('plugin_fields_%sdropdowns_id', $field->fields['name']); + $dropdown_class = PluginFieldsDropdown::getClassname($field->fields['name']); + $stored_ids = [ + $this->createItem($dropdown_class, ['name' => 'A'])->getID(), + $this->createItem($dropdown_class, ['name' => 'B'])->getID(), + ]; + $other_id = $this->createItem($dropdown_class, ['name' => 'C'])->getID(); + $computer = $this->createComputer(); + + $this->storeValues($container, $computer, [$dropdown_key => $stored_ids]); + + $data = PluginFieldsContainer::removeReadonlyValues( + $this->buildData($container, $computer, [$dropdown_key => [$other_id], '_' . $dropdown_key . '_defined' => 1]), + $computer, + ); + + $this->assertSame($stored_ids, $data[$dropdown_key]); + $this->assertArrayNotHasKey('_' . $dropdown_key . '_defined', $data); + } + + public function testDomBlockReadonlyFieldCannotBeOverwrittenFromItemForm(): void + { + $container = $this->createContainer('dom'); + $readonly_name = $this->createTextField($container, 'Locked', 1); + $editable_name = $this->createTextField($container, 'Editable', 0); + $computer = $this->createComputer(); + + $this->storeValues($container, $computer, [$readonly_name => 'stored', $editable_name => 'old']); + + $this->updateItem(Computer::class, $computer->getID(), [ + 'name' => 'Renamed', + $readonly_name => 'hacked', + $editable_name => 'new', + ], [$readonly_name, $editable_name]); + + $values = $this->getStoredValues($container, $computer); + $this->assertSame('stored', $values[$readonly_name]); + $this->assertSame('new', $values[$editable_name]); + } + + public function testDomBlockReadonlyFieldTakesItsDefaultOnItemCreation(): void + { + $container = $this->createContainer('dom'); + $readonly_name = $this->createTextField($container, 'Locked', 1, 'locked default'); + $editable_name = $this->createTextField($container, 'Editable', 0); + + $computer = $this->createItem(Computer::class, [ + 'name' => 'Computer ' . $this->getUniqueString(), + 'entities_id' => $this->entity_id, + $readonly_name => 'hacked', + $editable_name => 'new', + ], [$readonly_name, $editable_name]); + $this->assertInstanceOf(Computer::class, $computer); + + $values = $this->getStoredValues($container, $computer); + $this->assertSame('locked default', $values[$readonly_name]); + $this->assertSame('new', $values[$editable_name]); + } + + private function createContainer(string $type): PluginFieldsContainer + { + return $this->createFieldContainer([ + 'label' => 'Readonly ' . $type . ' ' . $this->getUniqueString(), + 'type' => $type, + 'itemtypes' => [Computer::class], + 'is_active' => 1, + 'entities_id' => $this->entity_id, + 'is_recursive' => 1, + ]); + } + + private function createTextField(PluginFieldsContainer $container, string $label, int $is_readonly, string $default_value = ''): string + { + $field = $this->createField([ + 'label' => $label, + 'type' => 'text', + PluginFieldsContainer::getForeignKeyField() => $container->getID(), + 'ranking' => 1, + 'is_active' => 1, + 'is_readonly' => $is_readonly, + 'default_value' => $default_value, + ]); + + return $field->fields['name']; + } + + private function createReadonlyOverride(PluginFieldsContainer $container, string $field_name, State $state): void + { + $this->createItem(PluginFieldsStatusOverride::class, [ + PluginFieldsContainer::getForeignKeyField() => $container->getID(), + 'plugin_fields_fields_id' => $this->fieldIdByName($field_name), + 'itemtype' => Computer::class, + 'states' => [$state->getID()], + 'is_readonly' => 1, + 'mandatory' => 0, + ], ['states', PluginFieldsContainer::getForeignKeyField()]); + } + + private function fieldIdByName(string $name): int + { + $field = new PluginFieldsField(); + $this->assertTrue($field->getFromDBByCrit(['name' => $name])); + + return $field->getID(); + } + + private function createComputer(): Computer + { + $computer = $this->createItem(Computer::class, [ + 'name' => 'Computer ' . $this->getUniqueString(), + 'entities_id' => $this->entity_id, + ]); + $this->assertInstanceOf(Computer::class, $computer); + + return $computer; + } + + private function buildData(PluginFieldsContainer $container, Computer $computer, array $values): array + { + return $values + [ + 'plugin_fields_containers_id' => $container->getID(), + 'itemtype' => Computer::class, + 'items_id' => $computer->getID(), + ]; + } + + private function storeValues(PluginFieldsContainer $container, Computer $computer, array $values): void + { + $this->assertTrue($container->updateFieldsValues($this->buildData($container, $computer, $values), Computer::class, false)); + } + + private function getStoredValues(PluginFieldsContainer $container, Computer $computer): array + { + $classname = PluginFieldsContainer::getClassname(Computer::class, $container->fields['name']); + $values_obj = new $classname(); + $this->assertTrue($values_obj->getFromDBByCrit(['items_id' => $computer->getID()])); + + return $values_obj->fields; + } +} diff --git a/tests/Units/ContainerTest.php b/tests/Units/ContainerTest.php index dd655418..e43ecdf5 100644 --- a/tests/Units/ContainerTest.php +++ b/tests/Units/ContainerTest.php @@ -42,6 +42,7 @@ use PluginFieldsContainer; use PluginFieldsDropdown; use PluginFieldsField; +use PluginFieldsToolbox; use Search; use Session; use Ticket; @@ -107,6 +108,47 @@ public function testAddWithoutItemtypesIsRejected(array $input): void $this->assertFalse($result); } + public static function provideMalformedItemtypes(): iterable + { + yield 'path separator' => ['itemtype' => '../Computer']; + yield 'leading digit' => ['itemtype' => '1Computer']; + yield 'whitespace' => ['itemtype' => 'Computer Model']; + } + + #[DataProvider('provideMalformedItemtypes')] + public function testAddWithMalformedItemtypeIsRejected(string $itemtype): void + { + $container = new PluginFieldsContainer(); + $result = $container->add([ + 'label' => 'Malformed itemtype', + 'type' => 'tab', + 'itemtypes' => [$itemtype], + 'is_active' => 1, + 'entities_id' => 0, + 'is_recursive' => 1, + ]); + + $this->assertFalse($result); + $this->hasSessionMessages(ERROR, ['At least one selected object is not a valid element type']); + } + + public function testAddWithNamespacedItemtypeSucceeds(): void + { + $definition = $this->initAssetDefinition('ns' . substr((string) $this->getUniqueString(), 0, 6)); + $asset_class = $definition->getAssetClassName(); + + $container = $this->createFieldContainer([ + 'label' => 'Ns ' . substr((string) $this->getUniqueString(), 0, 4), + 'type' => 'tab', + 'itemtypes' => [$asset_class], + 'is_active' => 1, + 'entities_id' => 0, + 'is_recursive' => 1, + ]); + + $this->assertContains($asset_class, PluginFieldsToolbox::decodeJSONItemtypes($container->fields['itemtypes'])); + } + public function testAddWithValidItemtypesSucceeds(): void { $container = $this->createFieldContainer([ diff --git a/tests/Units/ExportBlockAsYamlTest.php b/tests/Units/ExportBlockAsYamlTest.php new file mode 100644 index 00000000..37d66c0e --- /dev/null +++ b/tests/Units/ExportBlockAsYamlTest.php @@ -0,0 +1,125 @@ +. + * ------------------------------------------------------------------------- + * @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 Entity; +use Glpi\Tests\DbTestCase; +use Glpi\Tests\GLPITestCase; +use GlpiPlugin\Field\Tests\FieldTestTrait; +use PluginFieldsContainer; +use PluginFieldsProfile; + +require_once __DIR__ . '/../FieldTestCase.php'; + +final class ExportBlockAsYamlTest extends DbTestCase +{ + use FieldTestTrait; + + private int $root_entity_id; + + private int $child_entity_id; + + public function setUp(): void + { + GLPITestCase::setUp(); + $this->login(); + $this->root_entity_id = getItemByTypeName(Entity::class, '_test_root_entity', true); + $this->child_entity_id = getItemByTypeName(Entity::class, '_test_child_1', true); + $this->setEntity($this->root_entity_id, true); + } + + public function tearDown(): void + { + $this->tearDownFieldTest(); + GLPITestCase::tearDown(); + } + + public function testVisibleBlockIsExported(): void + { + $container = $this->createContainer($this->root_entity_id); + + $this->assertTrue(plugin_fields_exportBlockAsYaml($container->getID())); + $this->assertStringContainsString( + $container->getID() . '-' . Computer::class, + (string) file_get_contents(GLPI_TMP_DIR . '/fields_conf.yaml'), + ); + } + + public function testBlockWithoutProfileAccessIsOmitted(): void + { + $container = $this->createContainer($this->root_entity_id); + + $profile_right = new PluginFieldsProfile(); + $this->assertTrue($profile_right->getFromDBByCrit([ + 'profiles_id' => $_SESSION['glpiactiveprofile']['id'], + 'plugin_fields_containers_id' => $container->getID(), + ])); + $this->updateItem(PluginFieldsProfile::class, $profile_right->getID(), ['right' => 0]); + + $this->assertFalse(plugin_fields_exportBlockAsYaml($container->getID())); + } + + public function testBlockOutsideActiveEntitiesIsOmitted(): void + { + $container = $this->createContainer($this->child_entity_id); + + $this->setEntity($this->root_entity_id, false); + $this->assertFalse(plugin_fields_exportBlockAsYaml($container->getID())); + + $this->setEntity($this->child_entity_id, false); + $this->assertTrue(plugin_fields_exportBlockAsYaml($container->getID())); + } + + private function createContainer(int $entities_id): PluginFieldsContainer + { + $container = $this->createFieldContainer([ + 'label' => 'Export ' . $this->getUniqueString(), + 'type' => 'tab', + 'itemtypes' => [Computer::class], + 'is_active' => 1, + 'entities_id' => $entities_id, + 'is_recursive' => 0, + ]); + $this->createField([ + 'label' => 'Exported text', + 'type' => 'text', + PluginFieldsContainer::getForeignKeyField() => $container->getID(), + 'ranking' => 1, + 'is_active' => 1, + 'is_readonly' => 0, + ]); + + return $container; + } +} diff --git a/tests/Units/QuestionTypeAjaxControllerTest.php b/tests/Units/QuestionTypeAjaxControllerTest.php new file mode 100644 index 00000000..66808dfb --- /dev/null +++ b/tests/Units/QuestionTypeAjaxControllerTest.php @@ -0,0 +1,90 @@ +. + * ------------------------------------------------------------------------- + * @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 Entity; +use Glpi\Exception\Http\AccessDeniedHttpException; +use Glpi\Exception\Http\NotFoundHttpException; +use GlpiPlugin\Field\Tests\QuestionTypeTestCase; +use GlpiPlugin\Fields\Controller\QuestionTypeAjaxController; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Response; + +require_once __DIR__ . '/../QuestionTypeTestCase.php'; + +final class QuestionTypeAjaxControllerTest extends QuestionTypeTestCase +{ + public function testFormAdministratorGetsFieldContent(): void + { + $this->login(); + $this->setEntity($this->getTestRootEntity(true), true); + + $response = $this->invokeController(); + + $this->assertSame(Response::HTTP_OK, $response->getStatusCode()); + $this->assertStringContainsString($this->fields['glpi_item']->fields['label'], (string) $response->getContent()); + } + + public function testUserWithoutFormUpdateRightIsDenied(): void + { + $this->login('post-only', 'postonly'); + + $this->expectException(AccessDeniedHttpException::class); + $this->invokeController(); + } + + public function testBlockOutsideActiveEntitiesIsDenied(): void + { + $this->login(); + $this->setEntity(getItemByTypeName(Entity::class, '_test_child_1', true), false); + + $this->expectException(AccessDeniedHttpException::class); + $this->invokeController(); + } + + public function testUnknownBlockIsNotFound(): void + { + $this->login(); + $this->setEntity($this->getTestRootEntity(true), true); + + $this->expectException(NotFoundHttpException::class); + $this->invokeController(999999); + } + + private function invokeController(?int $block_id = null): Response + { + return (new QuestionTypeAjaxController())->__invoke( + Request::create('', 'POST', ['block_id' => $block_id ?? $this->block->getID()]), + ); + } +} From 737d8774b866efc08c4cf8caa4f3a7d79f692bab Mon Sep 17 00:00:00 2001 From: Stanislas Kita <7335054+stonebuzz@users.noreply.github.com> Date: Fri, 2 Oct 2026 09:52:05 +0200 Subject: [PATCH 2/3] Fix: lock block item types and type after creation --- CHANGELOG.md | 1 + front/container.form.php | 2 ++ inc/container.class.php | 32 +++++++++++++++++++---- tests/Units/ContainerTest.php | 49 +++++++++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 189e600a..886165ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Fix blocks export, block deletion, read-only fields, item fields and form editor field selection not applying the expected checks. - Fix read-only status overrides being resolved from the previous status instead of the submitted one. - Fix read-only fields of an "Insertion in form" block being overwritable from the item form. +- Fix a block's associated item types, type and tab being changeable after creation. - Remove obsolete FusionInventory inventory hook. ### Changed diff --git a/front/container.form.php b/front/container.form.php index c7440773..6266cf97 100644 --- a/front/container.form.php +++ b/front/container.form.php @@ -52,6 +52,8 @@ Html::redirect(PLUGINFIELDS_WEB_DIR . '/front/container.php'); } elseif (isset($_POST['update'])) { $container->check($_POST['id'], UPDATE); + // structural fields drive generated classes and tables; only migrations may change them + unset($_POST['itemtypes'], $_POST['type'], $_POST['subtype']); $container->update($_POST); Html::back(); } elseif (isset($_POST['update_fields_values'])) { diff --git a/inc/container.class.php b/inc/container.class.php index d77d1137..0bed98d9 100644 --- a/inc/container.class.php +++ b/inc/container.class.php @@ -624,9 +624,33 @@ public function prepareInputForUpdate($input) $input['label'] = PluginFieldsToolbox::sanitizeLabel((string) $input['label']); } + if (isset($input['itemtypes'])) { + $itemtypes = is_array($input['itemtypes']) + ? $input['itemtypes'] + : PluginFieldsToolbox::decodeJSONItemtypes((string) $input['itemtypes']); + if (!is_array($itemtypes) || $itemtypes === [] || !$this->areValidItemtypeNames($itemtypes)) { + Session::AddMessageAfterRedirect(__('At least one selected object is not a valid element type', 'fields'), false, ERROR); + + return false; + } + + $input['itemtypes'] = json_encode(array_values($itemtypes)); + } + return $input; } + private function areValidItemtypeNames(array $itemtypes): bool + { + foreach ($itemtypes as $itemtype) { + if (!is_string($itemtype) || preg_match('/^[A-Za-z_][A-Za-z0-9_\\\\]*$/', $itemtype) !== 1) { + return false; + } + } + + return true; + } + public function prepareInputForAdd($input) { if (empty($input['itemtypes'])) { @@ -646,12 +670,10 @@ public function prepareInputForAdd($input) $input['itemtypes'] = [$input['itemtypes']]; } - foreach ($input['itemtypes'] as $itemtype) { - if (!is_string($itemtype) || preg_match('/^[A-Za-z_][A-Za-z0-9_\\\\]*$/', $itemtype) !== 1) { - Session::AddMessageAfterRedirect(__('At least one selected object is not a valid element type', 'fields'), false, ERROR); + if (!$this->areValidItemtypeNames($input['itemtypes'])) { + Session::AddMessageAfterRedirect(__('At least one selected object is not a valid element type', 'fields'), false, ERROR); - return false; - } + return false; } if ($input['type'] === 'dom') { diff --git a/tests/Units/ContainerTest.php b/tests/Units/ContainerTest.php index e43ecdf5..6104bd40 100644 --- a/tests/Units/ContainerTest.php +++ b/tests/Units/ContainerTest.php @@ -38,6 +38,7 @@ use GlpiPlugin\Field\Tests\FieldTestTrait; use Laminas\Mail\Storage\Message; use MailCollector; +use Monitor; use PHPUnit\Framework\Attributes\DataProvider; use PluginFieldsContainer; use PluginFieldsDropdown; @@ -132,6 +133,54 @@ public function testAddWithMalformedItemtypeIsRejected(string $itemtype): void $this->hasSessionMessages(ERROR, ['At least one selected object is not a valid element type']); } + #[DataProvider('provideMalformedItemtypes')] + public function testUpdateWithMalformedItemtypeIsRejected(string $itemtype): void + { + $container = $this->createFieldContainer([ + 'label' => 'UpdMalformed ' . $this->getUniqueString(), + 'type' => 'tab', + 'itemtypes' => [Computer::class], + 'is_active' => 1, + 'entities_id' => 0, + 'is_recursive' => 1, + ]); + $original_itemtypes = $container->fields['itemtypes']; + + $result = $container->update([ + 'id' => $container->getID(), + 'itemtypes' => json_encode([$itemtype]), + ]); + + $this->assertFalse($result); + $this->hasSessionMessages(ERROR, ['At least one selected object is not a valid element type']); + $container->getFromDB($container->getID()); + $this->assertSame($original_itemtypes, $container->fields['itemtypes']); + } + + public function testUpdateWithValidItemtypesReencodesThem(): void + { + $container = $this->createFieldContainer([ + 'label' => 'UpdValid ' . $this->getUniqueString(), + 'type' => 'tab', + 'itemtypes' => [Computer::class], + 'is_active' => 1, + 'entities_id' => 0, + 'is_recursive' => 1, + ]); + + $result = $container->update([ + 'id' => $container->getID(), + 'itemtypes' => json_encode([Computer::class, Monitor::class]), + ]); + + $this->assertTrue($result); + $container->getFromDB($container->getID()); + $this->assertSame( + [Computer::class, Monitor::class], + PluginFieldsToolbox::decodeJSONItemtypes($container->fields['itemtypes']), + ); + } + public function testAddWithNamespacedItemtypeSucceeds(): void { $definition = $this->initAssetDefinition('ns' . substr((string) $this->getUniqueString(), 0, 6)); From e0e4e602af21050f38302328e976ab3a6c777c84 Mon Sep 17 00:00:00 2001 From: Stanislas Kita <7335054+stonebuzz@users.noreply.github.com> Date: Fri, 2 Oct 2026 09:58:39 +0200 Subject: [PATCH 3/3] Fix: remove FusionInventory rule hooks and check block right --- CHANGELOG.md | 2 +- hook.php | 79 ------------------- setup.php | 4 - src/Controller/QuestionTypeAjaxController.php | 6 +- .../Units/QuestionTypeAjaxControllerTest.php | 17 ++++ 5 files changed, 23 insertions(+), 85 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 886165ca..23130b95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Fix read-only status overrides being resolved from the previous status instead of the submitted one. - Fix read-only fields of an "Insertion in form" block being overwritable from the item form. - Fix a block's associated item types, type and tab being changeable after creation. -- Remove obsolete FusionInventory inventory hook. +- Remove obsolete FusionInventory integration. ### Changed diff --git a/hook.php b/hook.php index 6b5c4423..d9cbb497 100644 --- a/hook.php +++ b/hook.php @@ -234,85 +234,6 @@ function plugin_fields_MassiveActionsFieldsDisplay($options = []) return false; } - -/**** RULES ENGINE ****/ - -/** - * - * Actions for rules - * @since 0.84 - * @param array $params input data - * @return array an array of actions - */ -function plugin_fields_getRuleActions($params = []) -{ - $actions = []; - - if ($params['rule_itemtype'] === 'PluginFusioninventoryTaskpostactionRule') { - $options = PluginFieldsContainer::getAddSearchOptions('Computer'); - foreach ($options as $option) { - $actions[$option['linkfield']]['name'] = $option['name']; - $actions[$option['linkfield']]['type'] = $option['pfields_type']; - if ($option['pfields_type'] == 'dropdown') { - $actions[$option['linkfield']]['table'] = $option['table']; - } - } - } - - return $actions; -} - - -function plugin_fields_rule_matched($params = []) -{ - /** @var DBmysql $DB */ - global $DB; - - $container = new PluginFieldsContainer(); - - if (class_exists('PluginFusioninventoryAgent') && $params['sub_type'] == 'PluginFusioninventoryTaskpostactionRule') { - $agent = new PluginFusioninventoryAgent(); - - if (isset($params['input']['plugin_fusioninventory_agents_id'])) { - foreach ($params['output'] as $field => $value) { - // check if current field is in a tab container - $iterator = $DB->request([ - 'SELECT' => 'glpi_plugin_fields_containers.id', - 'FROM' => 'glpi_plugin_fields_containers', - 'LEFT JOIN' => [ - 'glpi_plugin_fields_fields' => [ - 'FKEY' => [ - 'glpi_plugin_fields_containers' => 'id', - 'glpi_plugin_fields_fields' => 'plugin_fields_containers_id', - ], - ], - ], - 'WHERE' => [ - 'glpi_plugin_fields_fields.name' => $field, - ], - ]); - if (count($iterator) > 0) { - $data = $iterator->current(); - - //retrieve computer - $agents_id = $params['input']['plugin_fusioninventory_agents_id']; - $agent->getFromDB($agents_id); - - // update current field - $container->updateFieldsValues( - [ - 'plugin_fields_containers_id' => $data['id'], - $field => $value, - 'items_id' => $agent->fields['computers_id'], - ], - Computer::getType(), - ); - } - } - } - } -} - function plugin_fields_giveItem($itemtype, $ID, $data, $num) { $searchopt = Search::getOptions($itemtype); diff --git a/setup.php b/setup.php index 79337df6..ace4e0d7 100644 --- a/setup.php +++ b/setup.php @@ -154,10 +154,6 @@ function plugin_init_fields() } } - // complete rule engine - $PLUGIN_HOOKS['use_rules']['fields'] = ['PluginFusioninventoryTaskpostactionRule']; - $PLUGIN_HOOKS['rule_matched']['fields'] = 'plugin_fields_rule_matched'; - if (isset($_SESSION['glpiactiveentities'])) { // add link in plugin page $PLUGIN_HOOKS['config_page']['fields'] = 'front/container.php'; diff --git a/src/Controller/QuestionTypeAjaxController.php b/src/Controller/QuestionTypeAjaxController.php index 8a73e43f..7fef0796 100644 --- a/src/Controller/QuestionTypeAjaxController.php +++ b/src/Controller/QuestionTypeAjaxController.php @@ -36,6 +36,7 @@ use Glpi\Form\Form; use PluginFieldsContainer; use PluginFieldsField; +use PluginFieldsProfile; use PluginFieldsQuestionType; use Session; use Symfony\Component\HttpFoundation\Request; @@ -69,7 +70,10 @@ public function __invoke(Request $request): Response throw new NotFoundHttpException(); } - if (!Session::haveAccessToEntity($block->fields['entities_id'], (bool) $block->fields['is_recursive'])) { + if ( + !Session::haveAccessToEntity($block->fields['entities_id'], (bool) $block->fields['is_recursive']) + || PluginFieldsProfile::getRightOnContainer((int) ($_SESSION['glpiactiveprofile']['id'] ?? 0), $block->getID()) < READ + ) { throw new AccessDeniedHttpException(); } diff --git a/tests/Units/QuestionTypeAjaxControllerTest.php b/tests/Units/QuestionTypeAjaxControllerTest.php index 66808dfb..a0eb7c9e 100644 --- a/tests/Units/QuestionTypeAjaxControllerTest.php +++ b/tests/Units/QuestionTypeAjaxControllerTest.php @@ -37,6 +37,7 @@ use Glpi\Exception\Http\NotFoundHttpException; use GlpiPlugin\Field\Tests\QuestionTypeTestCase; use GlpiPlugin\Fields\Controller\QuestionTypeAjaxController; +use PluginFieldsProfile; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -72,6 +73,22 @@ public function testBlockOutsideActiveEntitiesIsDenied(): void $this->invokeController(); } + public function testBlockWithoutProfileReadRightIsDenied(): void + { + $this->login(); + $this->setEntity($this->getTestRootEntity(true), true); + + $profile_right = new PluginFieldsProfile(); + $this->assertTrue($profile_right->getFromDBByCrit([ + 'profiles_id' => $_SESSION['glpiactiveprofile']['id'], + 'plugin_fields_containers_id' => $this->block->getID(), + ])); + $this->updateItem(PluginFieldsProfile::class, $profile_right->getID(), ['right' => 0]); + + $this->expectException(AccessDeniedHttpException::class); + $this->invokeController(); + } + public function testUnknownBlockIsNotFound(): void { $this->login();