From 759a1f9c47e9ce6f720635f85bb15d841d0c4cfb Mon Sep 17 00:00:00 2001 From: "Oscar B." Date: Wed, 26 Aug 2026 20:17:07 +0200 Subject: [PATCH 1/8] Update README.md Signed-off-by: Oscar B. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 02fbc2c..67fc02b 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ [![X](https://img.shields.io/twitter/follow/ticgalcom?style=flat-square&logo=x&label=Follow)](https://twitter.com/ticgalcom) [![Web](https://img.shields.io/badge/Web-TICGAL-blue.svg?style=flat-square)](https://tic.gal/) [![Localazy](https://img.shields.io/badge/Translate-Localazy-cyan)](https://localazy.com/p/more-groups) -[![Manual](https://img.shields.io/badge/Manual-docs.tic.gal-blue.svg?style=flat-square)](https://docs.tic.gal/books/more-groups) +[![Manual](https://img.shields.io/badge/Manual-Manuals-blue.svg?style=flat-square)](https://docs.tic.gal/books/more-groups) [![Marketplace](https://img.shields.io/badge/GLPI-Marketplace-orange.svg?style=flat-square)](https://plugins.glpi-project.org/#/plugins/moregroups) Fast group-membership management for GLPI: deactivate a group member without From c9a26a48f73c671e3e42457a8ade019e1b28951b Mon Sep 17 00:00:00 2001 From: "Oscar B." Date: Wed, 26 Aug 2026 20:17:41 +0200 Subject: [PATCH 2/8] Update badge link from 'Manual' to 'Doc' Signed-off-by: Oscar B. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 67fc02b..25a92b7 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ [![X](https://img.shields.io/twitter/follow/ticgalcom?style=flat-square&logo=x&label=Follow)](https://twitter.com/ticgalcom) [![Web](https://img.shields.io/badge/Web-TICGAL-blue.svg?style=flat-square)](https://tic.gal/) [![Localazy](https://img.shields.io/badge/Translate-Localazy-cyan)](https://localazy.com/p/more-groups) -[![Manual](https://img.shields.io/badge/Manual-Manuals-blue.svg?style=flat-square)](https://docs.tic.gal/books/more-groups) +[![Manual](https://img.shields.io/badge/Doc-Manuals-blue.svg?style=flat-square)](https://docs.tic.gal/books/more-groups) [![Marketplace](https://img.shields.io/badge/GLPI-Marketplace-orange.svg?style=flat-square)](https://plugins.glpi-project.org/#/plugins/moregroups) Fast group-membership management for GLPI: deactivate a group member without From d90c5374b3de9d41c7301d73174f2f287f7c8270 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Garc=C3=ADa?= Date: Mon, 31 Aug 2026 13:19:56 +0200 Subject: [PATCH 3/8] fix security problems --- inc/group.class.php | 112 ++++++++++++----------- setup.php | 2 +- src/Controller/GroupActionController.php | 2 +- 3 files changed, 61 insertions(+), 55 deletions(-) diff --git a/inc/group.class.php b/inc/group.class.php index 44d4c0c..3d7e53e 100644 --- a/inc/group.class.php +++ b/inc/group.class.php @@ -56,7 +56,10 @@ public function getForbiddenStandardMassiveAction() public function getSpecificMassiveActions($checkitem = null) { $actions = []; - $actions[__CLASS__ . MassiveAction::CLASS_ACTION_SEPARATOR . 'activate'] = __('Activate users', 'moregroups'); + // Solo ofrecer la acción si el usuario tiene permiso de modificación + if (Group_User::canUpdate()) { + $actions[__CLASS__ . MassiveAction::CLASS_ACTION_SEPARATOR . 'activate'] = __('Activate users', 'moregroups'); + } $actions += parent::getSpecificMassiveActions($checkitem); return $actions; @@ -77,24 +80,17 @@ static function showMassiveActionsSubForm(MassiveAction $ma) return parent::showMassiveActionsSubForm($ma); } - private static function canAccessGroupEntity($groups_id) - { - $group = new Group(); - return $group->can($groups_id, READ); - } - static function processMassiveActionsForOneItemtype(MassiveAction $ma, CommonDBTM $item, array $ids) { - global $DB; - switch ($ma->getAction()) { case 'deactivate': foreach ($ids as $id) { if (!$item->getFromDB($id)) { $ma->itemDone($item->getType(), $id, MassiveAction::ACTION_KO); $ma->addMessage($item->getErrorMessage(ERROR_NOT_FOUND)); - } elseif (!self::canAccessGroupEntity($item->fields['groups_id'])) { - $ma->itemDone($item->getType(), $id, MassiveAction::ACTION_KO); + // CORRECCIÓN VULNERABILIDAD ALTA: Validar permiso de UPDATE en la relación Group_User + } elseif (!$item->can($id, UPDATE)) { + $ma->itemDone($item->getType(), $id, MassiveAction::ACTION_NORIGHT); $ma->addMessage($item->getErrorMessage(ERROR_RIGHT)); } else { $input = $item->fields; @@ -115,19 +111,23 @@ static function processMassiveActionsForOneItemtype(MassiveAction $ma, CommonDBT } } return true; + case 'activate': foreach ($ids as $id) { if (!$item->getFromDB($id)) { $ma->itemDone($item->getType(), $id, MassiveAction::ACTION_KO); $ma->addMessage($item->getErrorMessage(ERROR_NOT_FOUND)); - } elseif (!self::canAccessGroupEntity($item->fields['groups_id'])) { - $ma->itemDone($item->getType(), $id, MassiveAction::ACTION_KO); + // CORRECCIÓN VULNERABILIDAD ALTA: Exigir permiso de UPDATE en el registro y en Group_User + } elseif (!$item->can($id, UPDATE) || !Group_User::canUpdate()) { + $ma->itemDone($item->getType(), $id, MassiveAction::ACTION_NORIGHT); $ma->addMessage($item->getErrorMessage(ERROR_RIGHT)); } else { $input = $item->fields; unset($input['id']); $group_user = new Group_User(); if ($group_user->add($input)) { + // Purga el registro de la tabla del plugin tras reactivar exitosamente al usuario + $item->delete(['id' => $id], true); $ma->itemDone($item->getType(), $id, MassiveAction::ACTION_OK); } else { $ma->itemDone($item->getType(), $id, MassiveAction::ACTION_KO); @@ -144,25 +144,22 @@ public static function showDeactivated($item) { global $DB; - $query = [ - 'FROM' => self::getTable(), - ]; - if ($item->getType() == 'Group') { - $query['WHERE'] = [ - 'AND' => [ - 'groups_id' => $item->getID(), - ], - ]; + if ($item->getType() != 'Group') { + return false; } $ID = $item->getID(); - if ( - !User::canView() - || !$item->can($ID, READ) - ) { + if (!User::canView() || !$item->can($ID, READ)) { return false; } + $query = [ + 'FROM' => self::getTable(), + 'WHERE' => [ + 'groups_id' => $ID, + ], + ]; + $canedit = Group_User::canUpdate(); $rand = mt_rand(); @@ -189,7 +186,7 @@ public static function showDeactivated($item) 'delegatee' => $row['is_userdelegate'] ? "" : '', ]; if ($canedit) { - $entry['actions'] = ""; + $entry['actions'] = ""; } $entries[] = $entry; } @@ -228,19 +225,19 @@ public static function showDeactivated($item) ]); if ($canedit) { - $label = _sx('button', 'Deactivate user', 'moregroups'); + $label = htmlspecialchars(_sx('button', 'Deactivate user', 'moregroups'), ENT_QUOTES); $script = <<"); - }); - }); - JAVASCRIPT; + $(document).ready(function() { + $("input[name^='item[Group_User]']").each(function() { + var name = $(this).attr('name'); + const myarray = name.split('['); + if (myarray.length >= 3) { + name = myarray[2].split(']')[0]; + $(this).parent().parent().append(""); + } + }); + }); + JAVASCRIPT; echo Html::scriptBlock($script); } @@ -267,20 +264,29 @@ static function install(Migration $migration) if (!$DB->tableExists($table)) { $migration->displayMessage("Installing $table"); $query = "CREATE TABLE IF NOT EXISTS $table ( - `id` int {$default_key_sign} NOT NULL auto_increment, - `users_id` int unsigned NOT NULL DEFAULT '0', - `groups_id` int unsigned NOT NULL DEFAULT '0', - `is_dynamic` tinyint NOT NULL DEFAULT '0', - `is_manager` tinyint NOT NULL DEFAULT '0', - `is_userdelegate` tinyint NOT NULL DEFAULT '0', - PRIMARY KEY (`id`), - UNIQUE KEY `unicity` (`users_id`,`groups_id`), - KEY `groups_id` (`groups_id`), - KEY `is_dynamic` (`is_dynamic`), - KEY `is_manager` (`is_manager`), - KEY `is_userdelegate` (`is_userdelegate`) - ) ENGINE=InnoDB DEFAULT CHARSET={$default_charset} COLLATE={$default_collation} ROW_FORMAT=DYNAMIC;"; - $DB->doQuery($query) or die($DB->error()); + `id` int {$default_key_sign} NOT NULL auto_increment, + `users_id` int unsigned NOT NULL DEFAULT '0', + `groups_id` int unsigned NOT NULL DEFAULT '0', + `is_dynamic` tinyint NOT NULL DEFAULT '0', + `is_manager` tinyint NOT NULL DEFAULT '0', + `is_userdelegate` tinyint NOT NULL DEFAULT '0', + PRIMARY KEY (`id`), + UNIQUE KEY `unicity` (`users_id`,`groups_id`), + KEY `groups_id` (`groups_id`), + KEY `is_dynamic` (`is_dynamic`), + KEY `is_manager` (`is_manager`), + KEY `is_userdelegate` (`is_userdelegate`) + ) ENGINE=InnoDB DEFAULT CHARSET={$default_charset} COLLATE={$default_collation} ROW_FORMAT=DYNAMIC;"; + + // CORRECCIÓN SEVERIDAD BAJA: Manejo seguro de errores sin romper la ejecución PHP con die() + if (!$DB->doQuery($query)) { + \Session::addMessageAfterRedirect( + sprintf(__('Error creating table %s: %s', 'moregroups'), 'glpi_plugin_moregroups_groups', $DB->error()), + false, + ERROR + ); + return false; + } } } diff --git a/setup.php b/setup.php index 40907e5..e42071c 100644 --- a/setup.php +++ b/setup.php @@ -30,7 +30,7 @@ use Glpi\Plugin\Hooks; -define('PLUGIN_MOREGROUPS_VERSION', '2.0.1'); +define('PLUGIN_MOREGROUPS_VERSION', '2.0.2-beta1'); define('PLUGIN_MOREGROUPS_MIN_GLPI', '11.0.0'); define('PLUGIN_MOREGROUPS_MAX_GLPI', '11.0.99'); diff --git a/src/Controller/GroupActionController.php b/src/Controller/GroupActionController.php index 4a21d7f..94fa5a2 100644 --- a/src/Controller/GroupActionController.php +++ b/src/Controller/GroupActionController.php @@ -64,7 +64,7 @@ public function __invoke(Request $request): Response $this->deactivate((int) $rowid); } - return new RedirectResponse($request->headers->get('referer') ?? '/'); + return new RedirectResponse(\Html::getBackUrl()); } private function canAccessGroup(array $fields): bool From f83912e3eeef70d61d3a353c39986c6d358798c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Garc=C3=ADa?= Date: Tue, 1 Sep 2026 11:40:33 +0200 Subject: [PATCH 4/8] fix safety warnings --- setup.php | 2 +- tools/make_release.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/setup.php b/setup.php index e42071c..b9f4be1 100644 --- a/setup.php +++ b/setup.php @@ -30,7 +30,7 @@ use Glpi\Plugin\Hooks; -define('PLUGIN_MOREGROUPS_VERSION', '2.0.2-beta1'); +define('PLUGIN_MOREGROUPS_VERSION', '2.0.2'); define('PLUGIN_MOREGROUPS_MIN_GLPI', '11.0.0'); define('PLUGIN_MOREGROUPS_MAX_GLPI', '11.0.99'); diff --git a/tools/make_release.sh b/tools/make_release.sh index 7def059..e344ca9 100644 --- a/tools/make_release.sh +++ b/tools/make_release.sh @@ -196,7 +196,7 @@ else echo "Creating private release" PACKAGE_NAME="$PLUGINNAME-$RELEASE" fi -tar cjf "$PACKAGE_NAME.tar.bz2" $PLUGINNAME +tar cjf "glpi-$PACKAGE_NAME.tar.bz2" $PLUGINNAME cd $INIT_PWD From a7edb148aee79251b49375339becc387ddbdcfc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Garc=C3=ADa?= Date: Tue, 1 Sep 2026 11:44:13 +0200 Subject: [PATCH 5/8] fix safety warnings --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3e0a38..83881e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +## 2.0.2 - 01/09/2026 +### Bugs +- Fix safety warnings + ## 2.0.1 - 18/08/2026 ### Features - Published an online user manual From 5eed348aff92004815d8d0c404174938309f7b5e Mon Sep 17 00:00:00 2001 From: TicgalMaria Date: Tue, 1 Sep 2026 13:40:25 +0200 Subject: [PATCH 6/8] Update setup.php Signed-off-by: TicgalMaria --- setup.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.php b/setup.php index b9f4be1..3eb0a53 100644 --- a/setup.php +++ b/setup.php @@ -30,7 +30,7 @@ use Glpi\Plugin\Hooks; -define('PLUGIN_MOREGROUPS_VERSION', '2.0.2'); +define('PLUGIN_MOREGROUPS_VERSION', '2.0.3'); define('PLUGIN_MOREGROUPS_MIN_GLPI', '11.0.0'); define('PLUGIN_MOREGROUPS_MAX_GLPI', '11.0.99'); From 8dc4770afed3829fd4ba64ce54e397c89c6af2f1 Mon Sep 17 00:00:00 2001 From: TicgalMaria Date: Wed, 2 Sep 2026 14:41:06 +0200 Subject: [PATCH 7/8] Update src/Controller/GroupActionController.php Co-authored-by: Javier Lago Amoedo Signed-off-by: TicgalMaria --- src/Controller/GroupActionController.php | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/Controller/GroupActionController.php b/src/Controller/GroupActionController.php index 5780a72..087c046 100644 --- a/src/Controller/GroupActionController.php +++ b/src/Controller/GroupActionController.php @@ -70,7 +70,26 @@ public function __invoke(Request $request): Response $this->deactivate((int) $rowid); } - return new RedirectResponse(\Html::getBackUrl()); + // Html::getBackUrl() sanitizes the Referer header (rejects `javascript:`, etc.) + // but does not restrict it to this GLPI instance — an absolute external URL + // passes its check unchanged, and \Glpi\Toolbox\URL::isGLPIRelativeUrl() rejects + // *every* browser-sent Referer because browsers always send an absolute URL, not + // a relative one. Compare the host explicitly instead: allow same-origin targets, + // fall back to a known-safe internal page for anything else. + // + // A target is only trusted as "already relative" when it starts with a single + // `/` followed by neither `/` nor `\` — WHATWG URL parsing treats a leading + // `/\` or `//` the same as `//host/...` for http(s), so `parse_url()` reporting + // a null host is not enough on its own to call it same-origin. + $back_url = \Html::getBackUrl(); + $back_host = parse_url($back_url, PHP_URL_HOST); + $base_host = parse_url($CFG_GLPI['url_base'], PHP_URL_HOST); + $is_relative = $back_host === null && preg_match('#^/[^/\\\\]#', $back_url) === 1; + if (!$is_relative && $back_host !== $base_host) { + $back_url = $CFG_GLPI['root_doc'] . '/front/central.php'; + } + + return new RedirectResponse($back_url); } private function canAccessGroup(array $fields): bool From 9ca65cbe77362935c88ea2ed93c5f748795b0990 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Garc=C3=ADa?= Date: Wed, 2 Sep 2026 14:47:55 +0200 Subject: [PATCH 8/8] fix csrf --- src/Controller/GroupActionController.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Controller/GroupActionController.php b/src/Controller/GroupActionController.php index 087c046..7b07c4f 100644 --- a/src/Controller/GroupActionController.php +++ b/src/Controller/GroupActionController.php @@ -49,10 +49,6 @@ public function __invoke(Request $request): Response { global $CFG_GLPI; - if (!Session::validateCSRF($request->request->all())) { - throw new AccessDeniedHttpException(); - } - $rowaction = $request->request->get('rowaction'); $rowid = $request->request->get('rowid');