diff --git a/install/empty_data.php b/install/empty_data.php index 6f5e8a13361..7070a173330 100644 --- a/install/empty_data.php +++ b/install/empty_data.php @@ -6003,7 +6003,7 @@ public function getEmptyData(): array ], [ 'profiles_id' => self::PROFILE_TECHNICIAN, 'name' => 'changevalidation', - 'rights' => CREATE | PURGE, + 'rights' => READ | CREATE | PURGE, ], [ 'profiles_id' => self::PROFILE_SELF_SERVICE, 'name' => 'ticketvalidation', @@ -6307,15 +6307,15 @@ public function getEmptyData(): array ], [ 'profiles_id' => self::PROFILE_SUPER_ADMIN, 'name' => 'changevalidation', - 'rights' => CREATE | PURGE | CommonITILValidation::VALIDATE, + 'rights' => READ | CREATE | PURGE | CommonITILValidation::VALIDATE, ], [ 'profiles_id' => self::PROFILE_HOTLINER, 'name' => 'changevalidation', - 'rights' => CREATE | PURGE, + 'rights' => READ | CREATE | PURGE, ], [ 'profiles_id' => self::PROFILE_OBSERVER, 'name' => 'ticketvalidation', - 'rights' => PURGE | TicketValidation::CREATEREQUEST | TicketValidation::CREATEINCIDENT | TicketValidation::VALIDATEREQUEST | TicketValidation::VALIDATEINCIDENT, + 'rights' => READ | PURGE | TicketValidation::CREATEREQUEST | TicketValidation::CREATEINCIDENT | TicketValidation::VALIDATEREQUEST | TicketValidation::VALIDATEINCIDENT, ], [ 'profiles_id' => self::PROFILE_ADMIN, 'name' => 'computer', @@ -6618,15 +6618,15 @@ public function getEmptyData(): array ], [ 'profiles_id' => self::PROFILE_OBSERVER, 'name' => 'changevalidation', - 'rights' => CREATE | PURGE | CommonITILValidation::VALIDATE, + 'rights' => READ | CREATE | PURGE | CommonITILValidation::VALIDATE, ], [ 'profiles_id' => self::PROFILE_ADMIN, 'name' => 'changevalidation', - 'rights' => CREATE | PURGE | CommonITILValidation::VALIDATE, + 'rights' => READ | CREATE | PURGE | CommonITILValidation::VALIDATE, ], [ 'profiles_id' => self::PROFILE_ADMIN, 'name' => 'ticketvalidation', - 'rights' => PURGE | TicketValidation::CREATEREQUEST | TicketValidation::CREATEINCIDENT + 'rights' => READ | PURGE | TicketValidation::CREATEREQUEST | TicketValidation::CREATEINCIDENT | TicketValidation::VALIDATEREQUEST | TicketValidation::VALIDATEINCIDENT, ], [ 'profiles_id' => self::PROFILE_SUPER_ADMIN, @@ -6935,7 +6935,7 @@ public function getEmptyData(): array ], [ 'profiles_id' => self::PROFILE_SUPER_ADMIN, 'name' => 'ticketvalidation', - 'rights' => PURGE | TicketValidation::CREATEREQUEST | TicketValidation::CREATEINCIDENT | TicketValidation::VALIDATEREQUEST | TicketValidation::VALIDATEINCIDENT, + 'rights' => READ | PURGE | TicketValidation::CREATEREQUEST | TicketValidation::CREATEINCIDENT | TicketValidation::VALIDATEREQUEST | TicketValidation::VALIDATEINCIDENT, ], [ 'profiles_id' => self::PROFILE_HOTLINER, 'name' => 'computer', @@ -7229,7 +7229,7 @@ public function getEmptyData(): array ], [ 'profiles_id' => self::PROFILE_HOTLINER, 'name' => 'ticketvalidation', - 'rights' => PURGE | TicketValidation::CREATEREQUEST | TicketValidation::CREATEINCIDENT, + 'rights' => READ | PURGE | TicketValidation::CREATEREQUEST | TicketValidation::CREATEINCIDENT, ], [ 'profiles_id' => self::PROFILE_TECHNICIAN, 'name' => 'computer', @@ -7524,7 +7524,7 @@ public function getEmptyData(): array ], [ 'profiles_id' => self::PROFILE_TECHNICIAN, 'name' => 'ticketvalidation', - 'rights' => PURGE | TicketValidation::CREATEREQUEST | TicketValidation::CREATEINCIDENT, + 'rights' => READ | PURGE | TicketValidation::CREATEREQUEST | TicketValidation::CREATEINCIDENT, ], [ 'profiles_id' => self::PROFILE_SUPERVISOR, 'name' => 'computer', @@ -7755,7 +7755,7 @@ public function getEmptyData(): array ], [ 'profiles_id' => self::PROFILE_SUPERVISOR, 'name' => 'changevalidation', - 'rights' => CREATE | PURGE | CommonITILValidation::VALIDATE, + 'rights' => READ | CREATE | PURGE | CommonITILValidation::VALIDATE, ], [ 'profiles_id' => self::PROFILE_ADMIN, 'name' => 'state', @@ -7811,7 +7811,7 @@ public function getEmptyData(): array ], [ 'profiles_id' => self::PROFILE_SUPERVISOR, 'name' => 'ticketvalidation', - 'rights' => PURGE | TicketValidation::CREATEREQUEST | TicketValidation::CREATEINCIDENT | TicketValidation::VALIDATEREQUEST | TicketValidation::VALIDATEINCIDENT, + 'rights' => READ | PURGE | TicketValidation::CREATEREQUEST | TicketValidation::CREATEINCIDENT | TicketValidation::VALIDATEREQUEST | TicketValidation::VALIDATEINCIDENT, ], [ 'profiles_id' => self::PROFILE_READ_ONLY, 'name' => 'backup', @@ -7839,7 +7839,7 @@ public function getEmptyData(): array ], [ 'profiles_id' => self::PROFILE_READ_ONLY, 'name' => 'changevalidation', - 'rights' => self::RIGHT_NONE, + 'rights' => READ, ], [ 'profiles_id' => self::PROFILE_READ_ONLY, 'name' => 'computer', @@ -8095,7 +8095,7 @@ public function getEmptyData(): array ], [ 'profiles_id' => self::PROFILE_READ_ONLY, 'name' => 'ticketvalidation', - 'rights' => self::RIGHT_NONE, + 'rights' => READ, ], [ 'profiles_id' => self::PROFILE_READ_ONLY, 'name' => 'transfer', diff --git a/install/migrations/update_11.0.5_to_11.0.6/validation_read_right.php b/install/migrations/update_11.0.5_to_11.0.6/validation_read_right.php new file mode 100644 index 00000000000..e4c1fa6315b --- /dev/null +++ b/install/migrations/update_11.0.5_to_11.0.6/validation_read_right.php @@ -0,0 +1,138 @@ +. + * + * --------------------------------------------------------------------- + */ + +use Glpi\DBAL\QueryExpression; + +/** + * --------------------------------------------------------------------- + * + * GLPI - Gestionnaire Libre de Parc Informatique + * + * http://glpi-project.org + * + * @copyright 2015-2026 Teclib' and contributors. + * @licence https://www.gnu.org/licenses/gpl-3.0.html + * + * --------------------------------------------------------------------- + * + * LICENSE + * + * This file is part of GLPI. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + * --------------------------------------------------------------------- + */ + +/** @var DBmysql $DB */ +global $DB; + +// Add READ right to profiles that already have any ticketvalidation or changevalidation rights. +// Previously, the READ right was explicitly removed from CommonITILValidation::getRights(), +// making it impossible to view validations in read-only contexts (e.g. Object Lock mode). +foreach (['ticketvalidation', 'changevalidation'] as $right_name) { + $DB->update( + 'glpi_profilerights', + [ + 'rights' => new QueryExpression( + $DB->quoteName('rights') . ' | ' . READ + ), + ], + [ + 'name' => $right_name, + ['NOT' => ['rights' => 0]], + ] + ); +} + +// Ensure the Object Lock profile also has READ for validation rights, +// so validations remain visible when a ticket/change is viewed in read-only lock mode. +// This is needed separately from the above query because the lock profile may have +// rights = 0 (excluded by the NOT condition above) or may not have an entry at all. +$lock_profile_row = $DB->request([ + 'SELECT' => 'value', + 'FROM' => 'glpi_configs', + 'WHERE' => [ + 'name' => 'lock_lockprofile_id', + 'context' => 'core', + ], +])->current(); + +$lock_profile_id = (int) ($lock_profile_row['value'] ?? 0); +if ($lock_profile_id > 0) { + foreach (['ticketvalidation', 'changevalidation'] as $right_name) { + $existing = $DB->request([ + 'FROM' => 'glpi_profilerights', + 'WHERE' => [ + 'profiles_id' => $lock_profile_id, + 'name' => $right_name, + ], + ]); + + if (count($existing) > 0) { + $DB->update( + 'glpi_profilerights', + [ + 'rights' => new QueryExpression( + $DB->quoteName('rights') . ' | ' . READ + ), + ], + [ + 'profiles_id' => $lock_profile_id, + 'name' => $right_name, + ] + ); + } else { + $DB->insert( + 'glpi_profilerights', + [ + 'profiles_id' => $lock_profile_id, + 'name' => $right_name, + 'rights' => READ, + ] + ); + } + } +} diff --git a/src/CommonITILValidation.php b/src/CommonITILValidation.php index 0a76321216f..1b1c08d6f6e 100644 --- a/src/CommonITILValidation.php +++ b/src/CommonITILValidation.php @@ -180,6 +180,7 @@ public static function canView(): bool return Session::haveRightsOr( static::$rightname, array_merge( + [READ], static::getCreateRights(), static::getValidateRights(), static::getPurgeRights() @@ -1026,16 +1027,7 @@ private function showSummary(CommonITILObject $itil): void { global $CFG_GLPI, $DB; - if ( - !Session::haveRightsOr( - static::$rightname, - array_merge( - static::getCreateRights(), - static::getValidateRights(), - static::getPurgeRights() - ) - ) - ) { + if (!static::canView()) { return; } @@ -1795,7 +1787,7 @@ public function getRights($interface = 'central') { $values = parent::getRights(); - unset($values[UPDATE], $values[READ]); + unset($values[UPDATE]); $values[self::VALIDATE] = __('Validate'); diff --git a/src/TicketValidation.php b/src/TicketValidation.php index bc3c43574de..bca7584aa74 100644 --- a/src/TicketValidation.php +++ b/src/TicketValidation.php @@ -102,7 +102,7 @@ public function getRights($interface = 'central') { $values = parent::getRights(); - unset($values[UPDATE], $values[CREATE], $values[READ]); + unset($values[UPDATE], $values[CREATE]); $values[self::CREATEREQUEST] = ['short' => __('Create for request'), diff --git a/tests/functional/CommonITILValidationReadRightTest.php b/tests/functional/CommonITILValidationReadRightTest.php new file mode 100644 index 00000000000..adf5d75e758 --- /dev/null +++ b/tests/functional/CommonITILValidationReadRightTest.php @@ -0,0 +1,266 @@ +. + * + * --------------------------------------------------------------------- + */ + +namespace tests\units; + +use ChangeValidation; +use Glpi\Tests\DbTestCase; +use ObjectLock; +use Ticket; +use TicketValidation; + +class CommonITILValidationReadRightTest extends DbTestCase +{ + public function testTicketValidationCanViewWithReadRight(): void + { + $this->login(); + + $_SESSION['glpiactiveprofile'][TicketValidation::$rightname] = READ; + + $this->assertTrue(TicketValidation::canView()); + } + + public function testTicketValidationCanViewWithNoRight(): void + { + $this->login(); + + $_SESSION['glpiactiveprofile'][TicketValidation::$rightname] = 0; + + $this->assertFalse(TicketValidation::canView()); + } + + public function testChangeValidationCanViewWithReadRight(): void + { + $this->login(); + + $_SESSION['glpiactiveprofile'][ChangeValidation::$rightname] = READ; + + $this->assertTrue(ChangeValidation::canView()); + } + + public function testChangeValidationCanViewWithNoRight(): void + { + $this->login(); + + $_SESSION['glpiactiveprofile'][ChangeValidation::$rightname] = 0; + + $this->assertFalse(ChangeValidation::canView()); + } + + public function testTicketValidationGetRightsIncludesRead(): void + { + $this->login(); + + $validation = new TicketValidation(); + $rights = $validation->getRights(); + + $this->assertArrayHasKey(READ, $rights); + } + + public function testCommonITILValidationGetRightsIncludesRead(): void + { + $this->login(); + + $validation = new ChangeValidation(); + $rights = $validation->getRights(); + + $this->assertArrayHasKey(READ, $rights); + } + + public function testGetTimelineItemsIncludesValidationWithReadRight(): void + { + $this->login(); + $this->setEntity('Root entity', true); + + $ticket = $this->createItem(Ticket::class, [ + 'name' => 'Timeline validation test', + 'content' => 'Test content', + 'entities_id' => 0, + ]); + + $validation = $this->createItem(TicketValidation::class, [ + 'tickets_id' => $ticket->getID(), + 'entities_id' => 0, + 'itemtype_target' => 'User', + 'items_id_target' => 2, + ]); + + $this->assertGreaterThan(0, $validation->getID()); + + $ticket->getFromDB($ticket->getID()); + + // Simulate read-only access: only READ on ticketvalidation, READALL on ticket + $_SESSION['glpiactiveprofile'][TicketValidation::$rightname] = READ; + $_SESSION['glpiactiveprofile'][Ticket::$rightname] = Ticket::READALL; + + $timeline = $ticket->getTimelineItems(); + + $validation_items = array_filter( + $timeline, + fn($item) => ($item['type'] ?? '') === TicketValidation::class + ); + + $this->assertNotEmpty($validation_items); + } + + public function testGetTimelineItemsExcludesValidationWithNoRight(): void + { + $this->login(); + $this->setEntity('Root entity', true); + + $ticket = $this->createItem(Ticket::class, [ + 'name' => 'Timeline validation no-right test', + 'content' => 'Test content', + 'entities_id' => 0, + ]); + + $this->createItem(TicketValidation::class, [ + 'tickets_id' => $ticket->getID(), + 'entities_id' => 0, + 'itemtype_target' => 'User', + 'items_id_target' => 2, + ]); + + $ticket->getFromDB($ticket->getID()); + + // Same ticket right as the "includes" test, but no validation right + $_SESSION['glpiactiveprofile'][TicketValidation::$rightname] = 0; + $_SESSION['glpiactiveprofile'][Ticket::$rightname] = Ticket::READALL; + + $timeline = $ticket->getTimelineItems(); + + $validation_items = array_filter( + $timeline, + fn($item) => ($item['type'] ?? '') === TicketValidation::class + ); + + $this->assertEmpty($validation_items); + } + + public function testCanViewValidationAfterObjectLockSetReadOnlyProfile(): void + { + global $CFG_GLPI; + + $this->login(); + + // Save original values + $original_lock_use = $CFG_GLPI['lock_use_lock_item'] ?? 0; + $original_lock_profile_id = $CFG_GLPI['lock_lockprofile_id'] ?? 0; + $original_lock_profile = $CFG_GLPI['lock_lockprofile'] ?? null; + + // Ensure the user's active profile has validation rights (CREATE + READ) + $_SESSION['glpiactiveprofile'][TicketValidation::$rightname] + = READ | TicketValidation::CREATEREQUEST; + $_SESSION['glpiactiveprofile'][ChangeValidation::$rightname] + = READ | CREATE; + + // Simulate a lock profile that only allows READ + $CFG_GLPI['lock_use_lock_item'] = 1; + $CFG_GLPI['lock_lockprofile_id'] = 999; + $CFG_GLPI['lock_lockprofile'] = [ + TicketValidation::$rightname => READ, + ChangeValidation::$rightname => READ, + ]; + + try { + ObjectLock::setReadOnlyProfile(); + + // After applying the lock profile, the resulting rights should be: + // user's rights & lock profile rights = (READ | CREATEREQUEST) & READ = READ + $this->assertTrue( + TicketValidation::canView(), + 'TicketValidation should be viewable in Object Lock read-only mode' + ); + $this->assertTrue( + ChangeValidation::canView(), + 'ChangeValidation should be viewable in Object Lock read-only mode' + ); + + ObjectLock::revertProfile(); + } finally { + // Ensure profile is always reverted even if assertions fail + if (isset($_SESSION['glpilocksavedprofile'])) { + ObjectLock::revertProfile(); + } + + // Restore original config + $CFG_GLPI['lock_use_lock_item'] = $original_lock_use; + $CFG_GLPI['lock_lockprofile_id'] = $original_lock_profile_id; + $CFG_GLPI['lock_lockprofile'] = $original_lock_profile; + } + } + + public function testCannotViewValidationAfterObjectLockWithNoReadInLockProfile(): void + { + global $CFG_GLPI; + + $this->login(); + + // Save original values + $original_lock_use = $CFG_GLPI['lock_use_lock_item'] ?? 0; + $original_lock_profile_id = $CFG_GLPI['lock_lockprofile_id'] ?? 0; + $original_lock_profile = $CFG_GLPI['lock_lockprofile'] ?? null; + + // User has full validation rights + $_SESSION['glpiactiveprofile'][TicketValidation::$rightname] + = READ | TicketValidation::CREATEREQUEST | TicketValidation::VALIDATEREQUEST; + + // Lock profile without READ on validation — simulates the pre-fix state + $CFG_GLPI['lock_use_lock_item'] = 1; + $CFG_GLPI['lock_lockprofile_id'] = 999; + $CFG_GLPI['lock_lockprofile'] = [ + TicketValidation::$rightname => 0, + ]; + + try { + ObjectLock::setReadOnlyProfile(); + + // user rights & lock profile = (READ | CREATEREQUEST | VALIDATEREQUEST) & 0 = 0 + $this->assertFalse( + TicketValidation::canView(), + 'TicketValidation should NOT be viewable when lock profile has no READ right' + ); + + ObjectLock::revertProfile(); + } finally { + if (isset($_SESSION['glpilocksavedprofile'])) { + ObjectLock::revertProfile(); + } + + $CFG_GLPI['lock_use_lock_item'] = $original_lock_use; + $CFG_GLPI['lock_lockprofile_id'] = $original_lock_profile_id; + $CFG_GLPI['lock_lockprofile'] = $original_lock_profile; + } + } +}