Skip to content

Fix ticket observer can edit #1278

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Herafia wants to merge 11 commits into
base: main
Choose a base branch
from
+421 −6
Open

Fix ticket observer can edit #1278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
77609cb
Fix ticket observer can edit
Herafia Sep 29, 2026
4d2b1e7
fix CI
Herafia Sep 30, 2026
b9cd9f3
changelog
Herafia Sep 30, 2026
5558b6f
tests
Herafia Sep 30, 2026
09eb9fa
lint
Herafia Sep 30, 2026
9b45100
fix
Herafia Oct 6, 2026
e143ff9
test test
Herafia Oct 9, 2026
5866f58
fix test
Herafia Oct 9, 2026
3a5b8d3
test to fix tests
Herafia Oct 9, 2026
90a42b0
test fix tests
Herafia Oct 9, 2026
4f25e35
rector
Herafia Oct 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

- Fix ticket observers being able to edit and save additional fields they are not allowed to modify

## [1.25.0] - 2026-10-06

### Added
Expand Down
15 changes: 15 additions & 0 deletions inc/container.class.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2077,6 +2077,21 @@ 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'])
&& !$item->canUpdateItem()
) {
Comment on lines +2082 to +2085

@Rom1-B Rom1-B Sep 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scoping this to helpdesk + CommonITILObject reopens the bug for central-interface users without update right (e.g. an observer with only READASSIGN). The patch validated on ticket !46589 used !$item->canUpdateItem() unconditionally, matching the field.class.php display-side fix — Ticket::canUpdateItem() already handles both interfaces correctly.

Suggested change
if (
isset($_SESSION['glpiactiveprofile']['id'])
&& $_SESSION['glpiactiveprofile']['id'] != null
&& $item instanceof CommonITILObject
&& Session::getCurrentInterface() === 'helpdesk'
&& !$item->canRequesterUpdateItem()
) {
if (
isset($_SESSION['glpiactiveprofile']['id'])
&& $_SESSION['glpiactiveprofile']['id'] !== null
&& !$item->canUpdateItem()
) {

@stonebuzz stonebuzz Oct 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please take care of it

unset($item->input['_plugin_fields_data']);

@stonebuzz stonebuzz Oct 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Submitted values are dropped silently and the item update still succeeds, so the user gets no feedback. Acceptable since the UI renders fields read-only, but a Session::addMessageAfterRedirect() warning would make any future UI/server mismatch visible.

Session::addMessageAfterRedirect(
__('You do not have the right to modify the fields of this item', 'fields'),
false,
WARNING,
);
return true;
}

$data = $item->input['_plugin_fields_data'];
$data['itemtype'] = $item::class;
$data['entities_id'] = $item->isEntityAssign() ? $item->getEntityID() : 0;
Expand Down
6 changes: 3 additions & 3 deletions inc/field.class.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -1213,7 +1213,7 @@ public static function prepareHtmlFields(
return null;
}

$canedit = $right > READ;
$canedit = $right > READ && ($item->isNewItem() || $item->canUpdateItem());

@stonebuzz stonebuzz Sep 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new $canedit rule ($item->isNewItem() || $item->canUpdateItem()) is untested, both here and in showForTabContainer() at line 894. A rendering test should assert that the inputs are read-only for a helpdesk observer and editable on a new item.


// Fill status overrides if needed
if (in_array($item->getType(), PluginFieldsStatusOverride::getStatusItemtypes())) {
Expand Down Expand Up @@ -1349,7 +1349,7 @@ public static function prepareHtmlFields(
$value = is_array($decoded) ? $decoded : [];
}

if ($field['multiple'] && is_array($value)) {
if ($field['multiple']) {
// Flatten any nested arrays caused by corrupted DB data (double-encoded values)
// so that Dropdown::show() always receives a flat list of scalars.
$value = array_values(array_filter(
Expand Down
Loading
Loading