diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ecf7d67..1348685f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ 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 ticket observers being able to edit and save additional fields they are not allowed to modify ## [1.24.5] - 2026-09-11 diff --git a/inc/container.class.php b/inc/container.class.php index 8221cd1e..3ace8805 100644 --- a/inc/container.class.php +++ b/inc/container.class.php @@ -1909,6 +1909,24 @@ public static function preItemUpdate(CommonDBTM $item) { self::preItem($item); if (array_key_exists('_plugin_fields_data', $item->input)) { + // Only save plugin fields if the user can update this specific item. + // Automated contexts (cron jobs, API without active profile) bypass this check. + if ( + isset($_SESSION['glpiactiveprofile']['id']) + && $_SESSION['glpiactiveprofile']['id'] != null + && ( + // Central interface: no UPDATE right on this item type + !$item::canUpdate() + // Helpdesk interface: UPDATE right exists but user is not the requester (observer) + || ($item instanceof CommonITILObject + && Session::getCurrentInterface() === 'helpdesk' + && !$item->canRequesterUpdateItem()) + ) + ) { + unset($item->input['_plugin_fields_data']); + return true; + } + $data = $item->input['_plugin_fields_data']; $data['itemtype'] = $item::class; $data['entities_id'] = $item->isEntityAssign() ? $item->getEntityID() : 0; diff --git a/inc/field.class.php b/inc/field.class.php index 9a89e0d3..7eb741a8 100644 --- a/inc/field.class.php +++ b/inc/field.class.php @@ -891,7 +891,7 @@ public static function showForTabContainer($c_id, $item) return null; } - $canedit = $right > READ; + $canedit = $right > READ && ($item->isNewItem() || $item->canUpdateItem()); //get fields for this container $field_obj = new self(); @@ -1210,7 +1210,7 @@ public static function prepareHtmlFields( return null; } - $canedit = $right > READ; + $canedit = $right > READ && ($item->isNewItem() || $item->canUpdateItem()); // Fill status overrides if needed if (in_array($item->getType(), PluginFieldsStatusOverride::getStatusItemtypes())) { diff --git a/tests/Units/ContainerItemRightTest.php b/tests/Units/ContainerItemRightTest.php index ae31b33c..b3264a0c 100644 --- a/tests/Units/ContainerItemRightTest.php +++ b/tests/Units/ContainerItemRightTest.php @@ -32,6 +32,12 @@ namespace GlpiPlugin\Field\Tests\Units; +use Ticket; +use Profile; +use User; +use Ticket_User; +use CommonITILActor; +use CommonDBTM; use Computer; use Entity; use Glpi\Tests\DbTestCase; @@ -175,4 +181,137 @@ private function setRightOnContainer(int $containers_id, int $right): void ])); $this->updateItem(PluginFieldsProfile::class, $profile_right->getID(), ['right' => $right]); } + + /** + * Test rendering: helpdesk observer cannot edit fields + * An observer on a ticket should see fields rendered as readonly. + */ + public function testDomContainerRenderReadOnlyForHelpdeskObserver(): void + { + $this->login(); + $entity_id = getItemByTypeName(Entity::class, '_test_root_entity', true); + $this->setEntity($entity_id, true); + + $container = $this->createFieldContainer([ + 'label' => 'Observer Readonly Container', + 'type' => 'dom', + 'itemtypes' => [Ticket::class], + 'is_active' => 1, + 'entities_id' => $entity_id, + 'is_recursive' => 1, + ]); + $this->createField([ + 'label' => 'Observer Test Field', + 'type' => 'text', + PluginFieldsContainer::getForeignKeyField() => $container->getID(), + 'ranking' => 1, + 'is_active' => 1, + 'is_readonly' => 0, + ]); + + $ticket = $this->createItem(Ticket::class, [ + 'name' => 'Ticket for observer test', + 'content' => 'Test', + 'entities_id' => $entity_id, + ]); + + // Create a helpdesk observer role + $observer_profile = $this->createItem(Profile::class, [ + 'name' => 'Helpdesk_Observer_' . $this->getUniqueString(), + 'interface' => 'helpdesk', + ]); + // Grant write access on container + $this->setRightOnContainerForProfile($observer_profile->getID(), $container->getID(), READ); + + // Create observer user (not requester) + $observer_username = 'observer_' . $this->getUniqueString(); + $this->createItem(User::class, [ + 'name' => $observer_username, + 'password' => 'Test1234!', + 'password2' => 'Test1234!', + 'profiles_id' => $observer_profile->getID(), + '_profiles_id' => $observer_profile->getID(), + '_entities_id' => $entity_id, + '_is_recursive' => true, + ], ['password', 'password2']); + + // Add observer to ticket + $this->createItem(Ticket_User::class, [ + 'tickets_id' => $ticket->getID(), + 'users_id' => getItemByTypeName(User::class, $observer_username, true), + 'type' => CommonITILActor::OBSERVER, + ]); + + // Login as observer and render + $this->login($observer_username, 'Test1234!'); + $this->setEntity($entity_id, true); + + $html = $this->renderDomContainerForAny($container->getID(), $ticket); + + // Assert: field must be rendered with readonly attribute + $this->assertStringContainsString( + 'readonly', + $html, + 'Fields must be rendered as readonly for helpdesk observers.', + ); + } + + /** + * Test rendering: new item creation allows editing even for limited profiles + * When creating a new ticket, fields should remain editable regardless of observer role. + */ + public function testDomContainerRenderEditableOnNewTicketCreation(): void + { + $this->login(); + $entity_id = getItemByTypeName(Entity::class, '_test_root_entity', true); + $this->setEntity($entity_id, true); + + $container = $this->createFieldContainer([ + 'label' => 'New Ticket Container', + 'type' => 'dom', + 'itemtypes' => [Ticket::class], + 'is_active' => 1, + 'entities_id' => $entity_id, + 'is_recursive' => 1, + ]); + $this->createField([ + 'label' => 'New Ticket Field', + 'type' => 'text', + PluginFieldsContainer::getForeignKeyField() => $container->getID(), + 'ranking' => 1, + 'is_active' => 1, + 'is_readonly' => 0, + ]); + + // Create new (non-existent) ticket for rendering + $new_ticket = new Ticket(); + $new_ticket->fields['entities_id'] = $entity_id; + + $html = $this->renderDomContainerForAny($container->getID(), $new_ticket); + + // Assert: field must NOT be readonly when creating a new ticket + $this->assertStringNotContainsString( + 'readonly', + $html, + 'Fields must remain editable when creating a new ticket.', + ); + } + + private function renderDomContainerForAny(int $containers_id, CommonDBTM $item): string + { + ob_start(); + PluginFieldsField::showDomContainer($containers_id, $item); + return (string) ob_get_clean(); + } + + private function setRightOnContainerForProfile(int $profile_id, int $containers_id, int $right): void + { + $profile_right = new PluginFieldsProfile(); + if ($profile_right->getFromDBByCrit([ + 'profiles_id' => $profile_id, + 'plugin_fields_containers_id' => $containers_id, + ])) { + $this->updateItem(PluginFieldsProfile::class, $profile_right->getID(), ['right' => $right]); + } + } }