Fix: check read rights before displaying additional fields

.gitignore

@@ -2,3 +2,5 @@ dist/
 vendor/
 .gh_token
 *.min.*
+tests/files/
+.phpunit.result.cache

CHANGELOG.md

@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ## [Unreleased]
 
 - 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
 

ajax/container.php

@@ -46,18 +46,28 @@
     $items_id      = (int) $_GET['items_id'];
     $type          = $_GET['type'];
     $subtype       = $_GET['subtype'];
-    $input         = $_GET['input'];
+    $input         = is_array($_GET['input'] ?? null) ? $_GET['input'] : [];
 
-    $item = new $itemtype();
-    if ($items_id > 0) {
-        if (!$item->getFromDB($items_id)) {
-            Response::sendError(404, 'Not Found');
-        }
+    if ($items_id > 0 && !PluginFieldsContainer::canReadTargetItem($itemtype, $items_id)) {
+        Response::sendError(403, 'Forbidden');
+        return;
+    }
+
+    $item = (new DbUtils())->getItemForItemtype($itemtype);
 
+    if (!$item instanceof CommonDBTM) {
+        Response::sendError(404, 'Not Found');
+        return;
+    }
+
+    if ($items_id > 0) {
         if (!$item->can($items_id, READ)) {
             Response::sendError(403, 'Forbidden');
             return;
         }
+    } elseif (!$item->can(0, CREATE, $input)) {
+        Response::sendError(403, 'Forbidden');
+        return;
     }
     $item->input = $input;
 

inc/container.class.php

@@ -1235,10 +1235,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);
     }
 
     /**

phpunit.xml

@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<phpunit
+    bootstrap="tests/bootstrap.php"
+    colors="true"
+>
+    <coverage>
+        <include>
+            <directory>inc</directory>
+        </include>
+    </coverage>
+
+    <testsuites>
+        <testsuite name="Tests">
+            <directory suffix="Test.php">tests</directory>
+        </testsuite>
+    </testsuites>
+</phpunit>

tests/FieldTestCase.php

@@ -0,0 +1,102 @@
+<?php
+
+/**
+ * -------------------------------------------------------------------------
+ * Fields plugin for GLPI
+ * -------------------------------------------------------------------------
+ *
+ * LICENSE
+ *
+ * This file is part of Fields.
+ *
+ * Fields 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 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Fields 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 Fields. If not, see <http://www.gnu.org/licenses/>.
+ * -------------------------------------------------------------------------
+ * @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
+ * -------------------------------------------------------------------------
+ */
+
+namespace GlpiPlugin\Field\Tests;
+
+use DBmysql;
+use PluginFieldsContainer;
+use PluginFieldsField;
+
+trait FieldTestTrait
+{
+    /** @var PluginFieldsContainer[] */
+    private static array $createdContainers = [];
+
+    /** @var PluginFieldsField[] */
+    private static array $createdFields = [];
+
+    public function tearDownFieldTest(): void
+    {
+        // Re-login to ensure we are logged in
+        $this->login();
+
+        array_map(
+            fn(PluginFieldsContainer $container) => $container->delete($container->fields, true),
+            self::$createdContainers,
+        );
+        self::$createdContainers = [];
+
+        array_map(
+            fn(PluginFieldsField $field) => $field->delete($field->fields, true),
+            self::$createdFields,
+        );
+        self::$createdFields = [];
+
+        /** @var DBmysql $DB */
+        global $DB;
+        $DB->clearSchemaCache();
+    }
+
+    public function createFieldContainer(array $inputs): PluginFieldsContainer
+    {
+        // Re-login to ensure we are logged in
+        $this->login();
+
+        $container = $this->createItem(PluginFieldsContainer::class, $inputs, ['itemtypes']);
+        self::$createdContainers[] = $container;
+
+        // Re-initialize fields plugin to register new container logic
+        plugin_init_fields();
+
+        /** @var DBmysql $DB */
+        global $DB;
+        $DB->clearSchemaCache();
+
+        return $container;
+    }
+
+    public function createField(array $inputs): PluginFieldsField
+    {
+        // Re-login to ensure we are logged in
+        $this->login();
+
+        $field = $this->createItem(PluginFieldsField::class, $inputs, ['allowed_values']);
+        self::$createdFields[] = $field;
+
+        // Re-initialize fields plugin to register new field logic
+        plugin_init_fields();
+
+        /** @var DBmysql $DB */
+        global $DB;
+        $DB->clearSchemaCache();
+
+        return $field;
+    }
+}

tests/Units/ContainerItemRightTest.php

@@ -0,0 +1,195 @@
+<?php
+
+/**
+ * -------------------------------------------------------------------------
+ * Fields plugin for GLPI
+ * -------------------------------------------------------------------------
+ *
+ * LICENSE
+ *
+ * This file is part of Fields.
+ *
+ * Fields 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 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Fields 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 Fields. If not, see <http://www.gnu.org/licenses/>.
+ * -------------------------------------------------------------------------
+ * @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 DbTestCase;
+use Entity;
+use 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();
+
+        global $CFG_GLPI;
+        $CFG_GLPI["event_loglevel"] = 0;
+
+        $this->login();
+    }
+
+    public function tearDown(): void
+    {
+        $this->tearDownFieldTest();
+        GLPITestCase::tearDown();
+    }
+
+    public function testCanUpdateTargetItemFollowsRightOnItem(): void
+    {
+        global $CFG_GLPI;
+        $CFG_GLPI["event_loglevel"] = 0;
+
+        $this->login();
+        $entity_id = getItemByTypeName(Entity::class, '_test_root_entity', true);
+        $this->setEntity($entity_id, true);
+        $computer = $this->createItem(Computer::class, [
+            'name'        => 'Computer ' . $this->getUniqueString(),
+            'entities_id' => $entity_id,
+        ]);
+
+        $this->assertTrue(PluginFieldsContainer::canUpdateTargetItem(Computer::class, $computer->getID()));
+
+        $this->login('post-only', 'postonly');
+        $this->setEntity($entity_id, true);
+
+        $this->assertFalse(PluginFieldsContainer::canUpdateTargetItem(Computer::class, $computer->getID()));
+    }
+
+    public function testCanUpdateTargetItemRejectsInvalidItemtype(): void
+    {
+        global $CFG_GLPI;
+        $CFG_GLPI["event_loglevel"] = 0;
+
+        $this->login();
+
+        $this->assertFalse(PluginFieldsContainer::canUpdateTargetItem('', 1));
+    }
+
+    public function testCanReadTargetItemFollowsRightOnItem(): void
+    {
+        global $CFG_GLPI;
+        $CFG_GLPI["event_loglevel"] = 0;
+
+        $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
+    {
+        global $CFG_GLPI;
+        $CFG_GLPI["event_loglevel"] = 0;
+
+        $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([
+            // Digits are spelled out in the generated system name, keep the label short and digit-free
+            'label'        => 'Dom container',
+            '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]);
+    }
+}