diff --git a/.phpstan-baseline.php b/.phpstan-baseline.php index 5efdebc9456..ae771ff3d96 100644 --- a/.phpstan-baseline.php +++ b/.phpstan-baseline.php @@ -3949,12 +3949,6 @@ 'count' => 1, 'path' => __DIR__ . '/src/Impact.php', ]; -$ignoreErrors[] = [ - 'message' => '#^PHPDoc tag @var with type class\\-string is not subtype of native type TKey of int\\|string\\.$#', - 'identifier' => 'varTag.nativeType', - 'count' => 2, - 'path' => __DIR__ . '/src/Impact.php', -]; $ignoreErrors[] = [ 'message' => '#^Right side of \\|\\| is always true\\.$#', 'identifier' => 'booleanOr.rightAlwaysTrue', diff --git a/ajax/impact.php b/ajax/impact.php index 58e37364676..0fefe271fc3 100644 --- a/ajax/impact.php +++ b/ajax/impact.php @@ -69,19 +69,12 @@ if (empty($itemtype)) { throw new BadRequestHttpException("Missing itemtype"); } - $icon = $CFG_GLPI["impact_asset_types"][$itemtype]; - // Execute search + // Execute search $assets = Impact::searchAsset($itemtype, json_decode($used), $filter, $page); foreach ($assets['items'] as $index => $item) { - $plugin_icon = Plugin::doHookFunction(Hooks::SET_ITEM_IMPACT_ICON, [ - 'itemtype' => $itemtype, - 'items_id' => $item['id'] - ]); - if ($plugin_icon && is_string($plugin_icon)) { - $icon = ltrim($plugin_icon, '/'); - } - $item['image'] = $CFG_GLPI['root_doc'] . '/' . $icon; + $item['image'] = Impact::getImpactIcon($itemtype, $item['id']); + $assets['items'][$index] = $item; } header('Content-Type: application/json'); diff --git a/phpunit/functional/Glpi/Toolbox/URLTest.php b/phpunit/functional/Glpi/Toolbox/URLTest.php index 42edb45f7a2..db086f39d5a 100644 --- a/phpunit/functional/Glpi/Toolbox/URLTest.php +++ b/phpunit/functional/Glpi/Toolbox/URLTest.php @@ -44,82 +44,110 @@ class URLTest extends \GLPITestCase public static function urlProvider(): iterable { yield [ - 'url' => null, - 'expected' => '', + 'url' => null, + 'sanitized' => '', + 'relative' => false, ]; yield [ - 'url' => '', - 'expected' => '', + 'url' => '', + 'sanitized' => '', + 'relative' => false, ]; // Javascript URL yield [ - 'url' => 'javascript:alert(1);', - 'expected' => '', + 'url' => 'javascript:alert(1);', + 'sanitized' => '', + 'relative' => false, ]; yield [ - 'url' => "java\nscript:alert(1);", - 'expected' => '', + 'url' => "java\nscript:alert(1);", + 'sanitized' => '', + 'relative' => false, ]; yield [ - 'url' => "j a v\t\ta\n s c \t ript :alert(1);", - 'expected' => '', + 'url' => "j a v\t\ta\n s c \t ript :alert(1);", + 'sanitized' => '', + 'relative' => false, ]; yield [ - 'url' => 'jAvAscrIPt:alert(1);', - 'expected' => '', + 'url' => 'jAvAscrIPt:alert(1);', + 'sanitized' => '', + 'relative' => false, ]; yield [ - 'url' => 'javascript:alert(1);" title="XSS!"', - 'expected' => '', + 'url' => 'javascript:alert(1);" title="XSS!"', + 'sanitized' => '', + 'relative' => false, ]; yield [ - 'url' => 'javascript:alert(1)', - 'expected' => '', + 'url' => 'javascript:alert(1)', + 'sanitized' => '', + 'relative' => false, ]; yield [ - 'url' => 'javascript://%0aalert();', - 'expected' => '', + 'url' => 'javascript://%0aalert();', + 'sanitized' => '', + 'relative' => false, ]; // Invalid URL yield [ - 'url' => 'ht tp://www.domain.tld/test', - 'expected' => '', + 'url' => 'ht tp://www.domain.tld/test', + 'sanitized' => '', + 'relative' => false, ]; yield [ - 'url' => 'http:/www.domain.tld/test', - 'expected' => '', + 'url' => 'http:/www.domain.tld/test', + 'sanitized' => '', + 'relative' => false, ]; yield [ - 'url' => '15//test', - 'expected' => '', + 'url' => '15//test', + 'sanitized' => '', + 'relative' => false, ]; // Sane URL yield [ - 'url' => 'http://www.domain.tld/test', - 'expected' => 'http://www.domain.tld/test', + 'url' => 'http://www.domain.tld/test', + 'sanitized' => 'http://www.domain.tld/test', + 'relative' => false, ]; yield [ - 'url' => '//hostname/path/to/file', - 'expected' => '//hostname/path/to/file', + 'url' => '//hostname/path/to/file', + 'sanitized' => '//hostname/path/to/file', + 'relative' => false, ]; yield [ - 'url' => '/test?abc=12', - 'expected' => '/test?abc=12', + 'url' => '/test?abc=12', + 'sanitized' => '/test?abc=12', + 'relative' => true, ]; yield [ - 'url' => '/', - 'expected' => '/', + 'url' => '/Path/To/Resource/15', + 'sanitized' => '/Path/To/Resource/15', + 'relative' => true, ]; + yield [ + 'url' => '/', + 'sanitized' => '/', + 'relative' => true, + ]; + } + + #[DataProvider('urlProvider')] + public function testSanitizeURL(?string $url, string $sanitized, bool $relative): void + { + $instance = new \Glpi\Toolbox\URL(); + $this->assertEquals($sanitized, $instance->sanitizeURL($url)); } #[DataProvider('urlProvider')] - public function testSanitizeURL(?string $url, string $expected): void + public function testIsGLPIRelativeUrl(?string $url, string $sanitized, bool $relative): void { $instance = new \Glpi\Toolbox\URL(); - $this->assertEquals($expected, $instance->sanitizeURL($url)); + $this->assertEquals($relative, $instance->isGLPIRelativeUrl((string) $url)); } public static function extractItemtypeFromUrlPathProvider(): iterable diff --git a/phpunit/functional/ImpactTest.php b/phpunit/functional/ImpactTest.php index 6962bad04ee..91d7831e009 100644 --- a/phpunit/functional/ImpactTest.php +++ b/phpunit/functional/ImpactTest.php @@ -37,11 +37,12 @@ use CommonDBTM; use Computer; +use Glpi\Plugin\Hooks; use ImpactCompound; use ImpactItem; use ImpactRelation; use Item_Ticket; -use PHPUnit\Framework\Attributes\DataProvider; +use Plugin; use Ticket; class ImpactTest extends \DbTestCase @@ -533,4 +534,55 @@ public function testFilterGraph() ); } } + + public function testGetImpactIconFromConfig(): void + { + /** + * @var array $CFG_GLPI + */ + global $CFG_GLPI; + + foreach (['', '/glpi'] as $root_doc) { + $CFG_GLPI['root_doc'] = $root_doc; + + foreach ($CFG_GLPI['impact_asset_types'] as $itemtype => $expected_icon) { + $this->assertSame($root_doc . $expected_icon, \Impact::getImpactIcon($itemtype)); + // By default, targetting a particular ID does not change the result. + $this->assertSame($root_doc . $expected_icon, \Impact::getImpactIcon($itemtype, 1)); + } + + $this->assertSame($root_doc . '/pics/impact/default.png', \Impact::getImpactIcon('NotAnAssetType')); + // By default, targetting a particular ID does not change the result. + $this->assertSame($root_doc . '/pics/impact/default.png', \Impact::getImpactIcon('NotAnAssetType', 1)); + } + } + + public function testGetImpactIconFromPluginHook(): void + { + /** + * @var array $CFG_GLPI + * @var array $PLUGIN_HOOKS + */ + global $CFG_GLPI, $PLUGIN_HOOKS; + + (new Plugin())->init(true); // The `tester` plugin must be considered as loaded/active. + + $PLUGIN_HOOKS[Hooks::SET_ITEM_IMPACT_ICON]['tester'] = function (array $params) { + if ($params['itemtype'] === 'PluginTesterMyAsset') { + return $params['items_id'] > 0 + ? sprintf('/plugins/tester/MyAsset/Picture/%d', $params['items_id']) + : '/plugins/tester/pics/myasset.png'; + } + return null; + }; + + foreach (['', '/glpi'] as $root_doc) { + $CFG_GLPI['root_doc'] = $root_doc; + + $this->assertSame($root_doc . '/plugins/tester/pics/myasset.png', \Impact::getImpactIcon('PluginTesterMyAsset')); + $this->assertSame($root_doc . '/plugins/tester/MyAsset/Picture/7', \Impact::getImpactIcon('PluginTesterMyAsset', 7)); + + $this->assertSame($root_doc . '/pics/impact/default.png', \Impact::getImpactIcon('PluginTesterAnotherAsset')); + } + } } diff --git a/src/Glpi/Toolbox/URL.php b/src/Glpi/Toolbox/URL.php index 54d755d2ec9..19f3de2e8e2 100644 --- a/src/Glpi/Toolbox/URL.php +++ b/src/Glpi/Toolbox/URL.php @@ -47,7 +47,7 @@ final class URL * * @return string */ - final public static function sanitizeURL(?string $url): string + public static function sanitizeURL(?string $url): string { if ($url === null) { return ''; @@ -75,6 +75,42 @@ final public static function sanitizeURL(?string $url): string return $url; } + /** + * Checks whether an URL can be considered as a valid GLPI relative URL. + * + * @param string $url + * + * @return bool + */ + public static function isGLPIRelativeUrl(string $url): bool + { + if ($url === '') { + return false; + } + + if (self::sanitizeURL($url) !== $url) { + return false; + } + + $parsed_url = parse_url($url); + + if ( + // URL is not parsable, it is invalid. + $parsed_url === false + // A relative URL should not contain a `scheme` or a `host` token + || array_key_exists('scheme', $parsed_url) + || array_key_exists('host', $parsed_url) + // A relative URL should contain a `path` token. + || !array_key_exists('path', $parsed_url) + // GLPI URLs are not supposed to contain special chars. + || preg_match('#[^a-z0-9_/\.-]#i', $parsed_url['path']) === 1 + ) { + return false; + } + + return true; + } + /** * Extract (lowercase) itemtype from a given URL path. * diff --git a/src/Impact.php b/src/Impact.php index 3b1a63aa78e..ec9aabef6e5 100644 --- a/src/Impact.php +++ b/src/Impact.php @@ -35,6 +35,7 @@ use Glpi\Application\View\TemplateRenderer; use Glpi\Plugin\Hooks; +use Glpi\Toolbox\URL; /** * @since 9.5.0 @@ -1011,37 +1012,29 @@ public static function printImpactNetworkContainer() ]); echo '
'; - $itemtypes = $CFG_GLPI["impact_asset_types"]; + $itemtypes = array_keys($CFG_GLPI["impact_asset_types"]); // Sort by translated itemtypes - uksort($itemtypes, function ($a, $b) { + usort($itemtypes, function ($a, $b) { /** @var class-string $a * @var class-string $b */ return strcasecmp($a::getTypeName(), $b::getTypeName()); }); - foreach ($itemtypes as $itemtype => $icon) { + foreach ($itemtypes as $itemtype) { /** @var class-string $itemtype */ // Do not display this itemtype if the user doesn't have READ rights if (!Session::haveRight($itemtype::$rightname, READ)) { continue; } - $plugin_icon = Plugin::doHookFunction(Hooks::SET_ITEM_IMPACT_ICON, [ - 'itemtype' => $itemtype, - 'items_id' => 0 - ]); - if ($plugin_icon && is_string($plugin_icon)) { - $icon = ltrim($plugin_icon, '/'); - } - - // Skip if not enabled + // Skip if not enabled if (!self::isEnabled($itemtype)) { continue; } - $icon = self::checkIcon($icon); + $icon = self::getImpactIcon($itemtype); echo '
'; - echo '

'; + echo '

'; echo "" . htmlescape($itemtype::getTypeName()) . "

"; echo '
'; // impact-side-filter-itemtypes-item } @@ -1270,26 +1263,56 @@ private static function buildGraphFromNode( } /** - * Check if the icon path is valid, if not return a fallback path + * Get the icon to be displayed for the given item. + * + * @param string $itemtype + * @param int|null $id * - * @param string $icon_path * @return string */ - private static function checkIcon(string $icon_path): string + public static function getImpactIcon(string $itemtype, ?int $id = null): string { - // Special case for images returned dynamicly - if (str_contains($icon_path, ".php")) { - return $icon_path; + /** + * @var array $CFG_GLPI + */ + global $CFG_GLPI; + + // First, try to get the icon from plugins + $plugin_icon = Plugin::doHookFunction( + Hooks::SET_ITEM_IMPACT_ICON, + [ + 'itemtype' => $itemtype, + 'items_id' => $id, + ] + ); + if (is_string($plugin_icon) && $plugin_icon !== '' && URL::isGLPIRelativeUrl($plugin_icon)) { + if (!str_starts_with($plugin_icon, '/')) { + // Fix paths declared without a leading `/`, as it was done before GLPI 11.0. + Toolbox::deprecated( + sprintf('Impact icon path `%s` must now be prefixed by a `/`.', $plugin_icon) + ); + $plugin_icon = '/' . $plugin_icon; + } + + return $CFG_GLPI['root_doc'] . $plugin_icon; } - // Check if icon exist on the filesystem - $file_path = GLPI_ROOT . "/$icon_path"; - if (file_exists($file_path) && is_file($file_path)) { - return $icon_path; + // Second, try to get the icon from the configuration entry + $icon = $CFG_GLPI['impact_asset_types'][$itemtype] ?? ''; + if (is_string($icon) && $icon !== '' && URL::isGLPIRelativeUrl($icon)) { + if (!str_starts_with($icon, '/')) { + // Fix paths declared without a leading `/`, as it was done before GLPI 11.0. + Toolbox::deprecated( + sprintf('Impact icon path `%s` must now be prefixed by a `/`.', $icon) + ); + $icon = '/' . $icon; + } + + return $CFG_GLPI['root_doc'] . $icon; } - // Fallback "default" icon - return "pics/impact/default.png"; + // Fallback to the default icon + return $CFG_GLPI['root_doc'] . '/pics/impact/default.png'; } /** @@ -1313,24 +1336,11 @@ private static function addNode(array &$nodes, CommonDBTM $item): bool return false; } - // Get web path to the image matching the itemtype from config - $image_name = $CFG_GLPI["impact_asset_types"][get_class($item)] ?? ""; - - $plugin_icon = Plugin::doHookFunction(Hooks::SET_ITEM_IMPACT_ICON, [ - 'itemtype' => get_class($item), - 'items_id' => $item->getID() - ]); - if ($plugin_icon && is_string($plugin_icon)) { - $image_name = ltrim($plugin_icon, '/'); - } - - $image_name = self::checkIcon($image_name); - // Define basic data of the new node $new_node = [ 'id' => $key, 'label' => $item->getFriendlyName(), - 'image' => $CFG_GLPI['root_doc'] . "/$image_name", + 'image' => self::getImpactIcon($item::class, $item->getID()), 'ITILObjects' => $item->getITILTickets(true), ]; diff --git a/src/autoload/CFG_GLPI.php b/src/autoload/CFG_GLPI.php index 98cb34d88e6..0347ab934ea 100644 --- a/src/autoload/CFG_GLPI.php +++ b/src/autoload/CFG_GLPI.php @@ -587,22 +587,22 @@ * Impact itemtypes enabled by default */ $CFG_GLPI["default_impact_asset_types"] = [ - Appliance::class => "pics/impact/appliance.png", - Cluster::class => "pics/impact/cluster.png", - Computer::class => "pics/impact/computer.png", - Datacenter::class => "pics/impact/datacenter.png", - DCRoom::class => "pics/impact/dcroom.png", - Domain::class => "pics/impact/domain.png", - Enclosure::class => "pics/impact/enclosure.png", - Monitor::class => "pics/impact/monitor.png", - NetworkEquipment::class => "pics/impact/networkequipment.png", - PDU::class => "pics/impact/pdu.png", - Peripheral::class => "pics/impact/peripheral.png", - Phone::class => "pics/impact/phone.png", - Printer::class => "pics/impact/printer.png", - Rack::class => "pics/impact/rack.png", - Software::class => "pics/impact/software.png", - DatabaseInstance::class => "pics/impact/databaseinstance.png", + Appliance::class => "/pics/impact/appliance.png", + Cluster::class => "/pics/impact/cluster.png", + Computer::class => "/pics/impact/computer.png", + Datacenter::class => "/pics/impact/datacenter.png", + DCRoom::class => "/pics/impact/dcroom.png", + Domain::class => "/pics/impact/domain.png", + Enclosure::class => "/pics/impact/enclosure.png", + Monitor::class => "/pics/impact/monitor.png", + NetworkEquipment::class => "/pics/impact/networkequipment.png", + PDU::class => "/pics/impact/pdu.png", + Peripheral::class => "/pics/impact/peripheral.png", + Phone::class => "/pics/impact/phone.png", + Printer::class => "/pics/impact/printer.png", + Rack::class => "/pics/impact/rack.png", + Software::class => "/pics/impact/software.png", + DatabaseInstance::class => "/pics/impact/databaseinstance.png", ]; /** @@ -610,26 +610,26 @@ * added in GLPI configuration */ $CFG_GLPI["impact_asset_types"] = $CFG_GLPI["default_impact_asset_types"] + [ - AuthLDAP::class => "pics/impact/authldap.png", - CartridgeItem::class => "pics/impact/cartridgeitem.png", - Contract::class => "pics/impact/contract.png", - CronTask::class => "pics/impact/crontask.png", - DeviceSimcard::class => "pics/impact/devicesimcard.png", - Entity::class => "pics/impact/entity.png", - Group::class => "pics/impact/group.png", - ITILCategory::class => "pics/impact/itilcategory.png", - Line::class => "pics/impact/line.png", - Location::class => "pics/impact/location.png", - MailCollector::class => "pics/impact/mailcollector.png", - Notification::class => "pics/impact/notification.png", - Profile::class => "pics/impact/profile.png", - Project::class => "pics/impact/project.png", - Rack::class => "pics/impact/rack.png", - SLM::class => "pics/impact/slm.png", - SoftwareLicense::class => "pics/impact/softwarelicense.png", - Supplier::class => "pics/impact/supplier.png", - User::class => "pics/impact/user.png", - Database::class => "pics/impact/database.png", + AuthLDAP::class => "/pics/impact/authldap.png", + CartridgeItem::class => "/pics/impact/cartridgeitem.png", + Contract::class => "/pics/impact/contract.png", + CronTask::class => "/pics/impact/crontask.png", + DeviceSimcard::class => "/pics/impact/devicesimcard.png", + Entity::class => "/pics/impact/entity.png", + Group::class => "/pics/impact/group.png", + ITILCategory::class => "/pics/impact/itilcategory.png", + Line::class => "/pics/impact/line.png", + Location::class => "/pics/impact/location.png", + MailCollector::class => "/pics/impact/mailcollector.png", + Notification::class => "/pics/impact/notification.png", + Profile::class => "/pics/impact/profile.png", + Project::class => "/pics/impact/project.png", + Rack::class => "/pics/impact/rack.png", + SLM::class => "/pics/impact/slm.png", + SoftwareLicense::class => "/pics/impact/softwarelicense.png", + Supplier::class => "/pics/impact/supplier.png", + User::class => "/pics/impact/user.png", + Database::class => "/pics/impact/database.png", ]; $CFG_GLPI['itemantivirus_types'] = ['Computer', 'Phone'];