Skip to content

Restrict webhooks by itemtype #21989

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

Merged
merged 2 commits into from
Nov 26, 2025
+55 −9
Merged

Restrict webhooks by itemtype #21989

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8936e1e
restrict webhooks by itemtype
cconard96 Nov 19, 2025
587123e
lint
cconard96 Nov 19, 2025
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
16 changes: 15 additions & 1 deletion src/Webhook.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,18 @@ public static function canDelete(): bool
return static::canUpdate();
}

public function canCreateItem(): bool
{
$itemtype = $this->fields['itemtype'];
return empty($itemtype) || (is_subclass_of($itemtype, CommonGLPI::class) && $itemtype::canView());
Copy link
Member

@cedric-anne cedric-anne Nov 19, 2025

Choose a reason for hiding this comment

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

Should be allow creation of hooks not related to any itemtype?

Copy link
Contributor Author

@cconard96 cconard96 Nov 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Not really, but those webhooks wouldn't be usable at all so from a restriction standpoint it isn't a big deal. There is already a block in Webhook::handleInput when the itemtype is missing. The empty check here was more to prevent errors in the subclass/canView check.

}

public function canUpdateItem(): bool
{
$itemtype = $this->fields['itemtype'];
return empty($itemtype) || (is_subclass_of($itemtype, CommonGLPI::class) && $itemtype::canView());
}

public function defineTabs($options = [])
{
$parent_tabs = parent::defineTabs();
Expand Down Expand Up @@ -456,7 +468,9 @@ public static function getItemtypesDropdownValues(): array
// Move leaf values to the keys and make the value the ::getTypeName
foreach ($values as $category => $itemtypes) {
foreach ($itemtypes as $i => $itemtype) {
$values[$category][$itemtype] = $itemtype::getTypeName(1);
if ($itemtype::canView()) {
$values[$category][$itemtype] = $itemtype::getTypeName(1);
}
unset($values[$category][$i]);
}
}
Expand Down
48 changes: 40 additions & 8 deletions tests/functional/WebhookTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
use Glpi\Api\HL\Controller\AbstractController;
use Glpi\Search\SearchOption;
use Psr\Log\LogLevel;
use Webhook;

class WebhookTest extends \DbTestCase
{
Expand All @@ -46,7 +47,7 @@ class WebhookTest extends \DbTestCase
*/
public function testWebhookTypesHaveIDOpt()
{
$supported = \Webhook::getItemtypesDropdownValues();
$supported = Webhook::getItemtypesDropdownValues();
$itemtypes = [];
foreach ($supported as $types) {
$itemtypes = array_merge($itemtypes, array_keys($types));
Expand Down Expand Up @@ -228,7 +229,7 @@ public function testWebhookHeaderTemplate()
public function testGetResultForPath()
{
$this->login();
/** @var \Webhook $webhook */
/** @var Webhook $webhook */
$webhook = $this->createItem('Webhook', [
'name' => 'Test webhook',
'entities_id' => $_SESSION['glpiactive_entity'],
Expand All @@ -249,7 +250,7 @@ public function testGetAPIItemtypeData()
$this->login();
$this->initAssetDefinition();

$supported_types = \Webhook::getAPIItemtypeData();
$supported_types = Webhook::getAPIItemtypeData();
foreach ($supported_types as $controller => $type_data) {
$this->assertTrue(is_subclass_of($controller, AbstractController::class));
foreach ($type_data as $category => $types) {
Expand All @@ -266,7 +267,7 @@ public function testGetAPIPath()
{
$this->login();

$webhook = new \Webhook();
$webhook = new Webhook();
$computer = getItemByTypeName('Computer', '_test_pc01');
$this->assertEquals('/Assets/Computer/' . $computer->getID(), $webhook->getAPIPath($computer));

Expand All @@ -283,7 +284,7 @@ public function testWithHLAPIDisabled(): void
global $CFG_GLPI;
$this->login();
$CFG_GLPI['enable_hlapi'] = 0;
/** @var \Webhook $webhook */
/** @var Webhook $webhook */
$webhook = $this->createItem('Webhook', [
'name' => 'Test webhook',
'entities_id' => $_SESSION['glpiactive_entity'],
Expand All @@ -301,12 +302,12 @@ public function testWithHLAPIDisabled(): void

public function testGetMonacoSuggestions()
{
$itemtypes = \Webhook::getItemtypesDropdownValues();
$itemtypes = Webhook::getItemtypesDropdownValues();

foreach ($itemtypes as $types) {
$this->assertIsArray($types);
foreach ($types as $itemtype => $label) {
$suggestions = \Webhook::getMonacoSuggestions($itemtype);
$suggestions = Webhook::getMonacoSuggestions($itemtype);
$this->assertNotEmpty($suggestions, "Missing suggestions for $itemtype");
}
}
Expand All @@ -316,7 +317,7 @@ public function testWebhookNotBlocker(): void
{
global $DB;

$this->createItem(\Webhook::class, [
$this->createItem(Webhook::class, [
'name' => 'Test webhook',
'entities_id' => $_SESSION['glpiactive_entity'],
'url' => 'http://localhost',
Expand Down Expand Up @@ -360,4 +361,35 @@ public function testWebhookNotBlocker(): void
]
);
}

public function testItemtypeDropdownExcludesNoReadItemtypes()
{
$this->login();
$this->assertContains('Computer', Webhook::getItemtypesDropdownValues()['Assets']);
$this->assertContains('Monitor', Webhook::getItemtypesDropdownValues()['Assets']);
$_SESSION['glpiactiveprofile']['computer'] = ALLSTANDARDRIGHT & ~READ;
$this->assertNotContains('Computer', Webhook::getItemtypesDropdownValues()['Assets']);
$this->assertContains('Monitor', Webhook::getItemtypesDropdownValues()['Assets']);
}

public function testCreateUpdateNoReadItemtypes()
{
$this->login();
$webhook = $this->createItem('Webhook', [
'name' => 'Test webhook',
'entities_id' => $_SESSION['glpiactive_entity'],
'url' => 'http://localhost',
'itemtype' => 'Computer',
'event' => 'new',
'is_active' => 1,
'use_default_payload' => 1,
]);
$this->assertTrue($webhook->canUpdateItem());
$_SESSION['glpiactiveprofile']['computer'] = ALLSTANDARDRIGHT & ~READ;
$this->assertFalse($webhook->canUpdateItem());

$this->assertFalse($webhook->canCreateItem());
$webhook->fields['itemtype'] = 'Monitor';
$this->assertTrue($webhook->canCreateItem());
}
}