Skip to content

Fix group assigned to a ticket without right #15

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
Lainow merged 4 commits into from
Sep 28, 2026
Merged

Fix group assigned to a ticket without right #15

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
af997cd
Fix group assigned to a ticket without right
Lainow Sep 24, 2026
af00b32
Update changelog
Lainow Sep 24, 2026
dd3e151
Merge branch 'main' into fix-group-cant-be-requester
RomainLvr Sep 25, 2026
4096948
Fix lints
Lainow Sep 28, 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 @@ -7,12 +7,18 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## [unreleased]

### Fixed

- Fixed an issue where a group could be added to a ticket even though it did not have the necessary permissions

## Add

- Add rector config

## [1.0.0-rc2]

### Fixed

- Fixed the issue where a ticket could be solved without a solution

## [1.0.0-rc1]
47 changes: 43 additions & 4 deletions src/Controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
use CommonITILValidation;
use Glpi\Application\View\TemplateRenderer;
use GlpiPlugin\Moreoptions\Config;
use Group;
use Group_Item;
use Group_Problem;
use Group_Ticket;
Expand Down Expand Up @@ -193,12 +194,41 @@ public static function addItemGroups(CommonDBTM $item): void
'type' => CommonITILActor::ASSIGN,
];

if (!$gitem->getFromDBByCrit($criteria)) {
if (
self::canGroupBeActor((int) $g['groups_id'], CommonITILActor::ASSIGN)
&& !$gitem->getFromDBByCrit($criteria)
) {
$gitem->add($criteria);
}
}
}

/**
* Check that the group exists and is allowed to be used for the given actor type
* (requester, observer or assigned), according to its "is_requester",
* "is_watcher" and "is_assign" flags.
*/
private static function canGroupBeActor(int $groups_id, int $actorType): bool
{
$field = match ($actorType) {
CommonITILActor::REQUESTER => 'is_requester',
CommonITILActor::OBSERVER => 'is_watcher',
CommonITILActor::ASSIGN => 'is_assign',
default => null,
};

if ($field === null || $groups_id <= 0) {
return false;
}

$group = new Group();
if (!$group->getFromDB($groups_id)) {
return false;
}

return (bool) $group->fields[$field];
}

/**
* Add groups for the given actor type based on the configuration
*/
Expand Down Expand Up @@ -256,7 +286,10 @@ private static function addGroupsForActorType(CommonDBTM $item, Config $moconfig
'type' => $actorType,
];

if (!$t_group->getFromDBByCrit($criteria)) {
if (
self::canGroupBeActor((int) $user->fields['groups_id'], $actorType)
&& !$t_group->getFromDBByCrit($criteria)
) {
$t_group->add($criteria + $escalade_options);
}
} else {
Expand All @@ -278,7 +311,10 @@ private static function addGroupsForActorType(CommonDBTM $item, Config $moconfig
'type' => $actorType,
];

if (!$t_group->getFromDBByCrit($criteria)) {
if (
self::canGroupBeActor((int) $ug['groups_id'], $actorType)
&& !$t_group->getFromDBByCrit($criteria)
) {
$t_group->add($criteria + $escalade_options);
}
}
Expand Down Expand Up @@ -674,7 +710,10 @@ public static function updateItemActors(CommonITILObject $item): CommonITILObjec
'type' => CommonITILActor::ASSIGN,
$itemIdField => $item->fields['id'],
];
if (!$group_link->getFromDBByCrit($criteria)) {
if (
self::canGroupBeActor((int) $category->fields['groups_id'], CommonITILActor::ASSIGN)
&& !$group_link->getFromDBByCrit($criteria)
) {
$group_link->add($criteria);
}
}
Expand Down
273 changes: 273 additions & 0 deletions tests/Units/ConfigTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2590,4 +2590,277 @@ public function testEscaladeConfigHandlesTechnicianGroup(
Config::escaladeConfigHandlesTechnicianGroup($escalade_config, $handled_by_behaviors),
);
}

/**
* Groups not allowed as requester must not be added as requester group
*/
public function testTakeTheRequesterGroupSkipsGroupsNotAllowedAsRequester(): void
{
$conf = $this->getCurrentConfig();

$this->assertTrue($this->updateTestConfig($conf, [
'entities_id' => 0,
'take_requester_group_ticket' => 2, // All
]));

$allowed_group = $this->createItem(
Group::class,
[
'name' => 'Requester allowed group',
'is_requester' => 1,
],
);
$forbidden_group = $this->createItem(
Group::class,
[
'name' => 'Requester forbidden group',
'is_requester' => 0,
],
);

$user = new User();
$this->assertTrue($user->getFromDBByCrit(['name' => 'glpi']));

foreach ([$allowed_group, $forbidden_group] as $group) {
$this->createItem(
Group_User::class,
[
'groups_id' => $group->getID(),
'users_id' => $user->getID(),
],
);
}

$ticket = $this->createItem(
Ticket::class,
[
'name' => 'Test ticket requester group not allowed',
'content' => 'Test content',
],
);

$this->createItem(
Ticket_User::class,
[
'tickets_id' => $ticket->getID(),
'users_id' => $user->getID(),
'type' => Ticket_User::REQUESTER,
],
);

$ticket_group = new Group_Ticket();
$groups = $ticket_group->find([
'tickets_id' => $ticket->getID(),
'type' => CommonITILActor::REQUESTER,
]);
$this->assertCount(1, $groups);
$this->assertEquals($allowed_group->getID(), current($groups)['groups_id']);

$this->assertTrue($this->updateTestConfig($conf, [
'entities_id' => 0,
'take_requester_group_ticket' => 0,
]));
}

/**
* Main group of the technician must not be added if it is not allowed as assigned
*/
public function testTakeTheTechnicianGroupSkipsGroupNotAllowedAsAssigned(): void
{
$conf = $this->getCurrentConfig();

$this->assertTrue($this->updateTestConfig($conf, [
'entities_id' => 0,
'take_technician_group_ticket' => 1, // Main group only
]));

$forbidden_group = $this->createItem(
Group::class,
[
'name' => 'Assign forbidden group',
'is_assign' => 0,
],
);

$user = new User();
$this->assertTrue($user->getFromDBByCrit(['name' => 'tech']));

$this->createItem(
Group_User::class,
[
'groups_id' => $forbidden_group->getID(),
'users_id' => $user->getID(),
],
);
$this->updateItem(
User::class,
$user->getID(),
[
'groups_id' => $forbidden_group->getID(),
],
);

$ticket = $this->createItem(
Ticket::class,
[
'name' => 'Test ticket technician group not allowed',
'content' => 'Test content',
],
);

$this->createItem(
Ticket_User::class,
[
'tickets_id' => $ticket->getID(),
'users_id' => $user->getID(),
'type' => Ticket_User::ASSIGN,
],
);

$ticket_group = new Group_Ticket();
$this->assertCount(0, $ticket_group->find([
'tickets_id' => $ticket->getID(),
'groups_id' => $forbidden_group->getID(),
]));

$this->assertTrue($this->updateTestConfig($conf, [
'entities_id' => 0,
'take_technician_group_ticket' => 0,
]));
}

/**
* Item groups not allowed as assigned must not be added to the ticket
*/
public function testTakeItemGroupsSkipsGroupNotAllowedAsAssigned(): void
{
$conf = $this->getCurrentConfig();

$this->assertTrue($this->updateTestConfig($conf, [
'entities_id' => 0,
'take_item_group_ticket' => 1,
]));

$allowed_group = $this->createItem(
Group::class,
[
'name' => 'Item group allowed',
'is_assign' => 1,
],
);
$forbidden_group = $this->createItem(
Group::class,
[
'name' => 'Item group forbidden',
'is_assign' => 0,
],
);

$computer = $this->createItem(
Computer::class,
[
'name' => 'Test computer group not allowed',
'entities_id' => 0,
],
);

foreach ([$allowed_group, $forbidden_group] as $group) {
$this->createItem(
Group_Item::class,
[
'items_id' => $computer->getID(),
'itemtype' => Computer::class,
'groups_id' => $group->getID(),
'type' => 1,
],
);
}

$ticket = $this->createItem(
Ticket::class,
[
'name' => 'Test ticket item group not allowed',
'content' => 'Test content',
],
);

$this->createItem(
Item_Ticket::class,
[
'tickets_id' => $ticket->getID(),
'items_id' => $computer->getID(),
'itemtype' => Computer::class,
],
);

$ticket_group = new Group_Ticket();
$groups = $ticket_group->find([
'tickets_id' => $ticket->getID(),
'type' => CommonITILActor::ASSIGN,
]);
$this->assertCount(1, $groups);
$this->assertEquals($allowed_group->getID(), current($groups)['groups_id']);

$this->assertTrue($this->updateTestConfig($conf, [
'entities_id' => 0,
'take_item_group_ticket' => 0,
]));
}

/**
* Category technical group not allowed as assigned must not be added to the ticket
*/
public function testUpdateTicketActorsOnCategoryChangeSkipsGroupNotAllowedAsAssigned(): void
{
$this->login();

$conf = $this->getCurrentConfig();

$this->assertTrue($this->updateTestConfig($conf, [
'entities_id' => 0,
'assign_technical_group_when_changing_category_ticket' => 1,
]));

$forbidden_group = $this->createItem(
Group::class,
[
'name' => 'Category group forbidden',
'is_assign' => 0,
],
);

$category = $this->createItem(
ITILCategory::class,
[
'name' => 'Test Category with forbidden group',
'groups_id' => $forbidden_group->getID(),
],
);

$ticket = $this->createItem(
Ticket::class,
[
'name' => 'Test ticket category group not allowed',
'content' => 'Test content',
],
);

$this->updateItem(
Ticket::class,
$ticket->getID(),
[
'itilcategories_id' => $category->getID(),
],
);

$ticket_group = new Group_Ticket();
$this->assertCount(0, $ticket_group->find([
'tickets_id' => $ticket->getID(),
'groups_id' => $forbidden_group->getID(),
]));

$this->assertTrue($this->updateTestConfig($conf, [
'assign_technical_group_when_changing_category_ticket' => 0,
]));
}
}
Loading