Fix active entities filtering

glpi-project · Mar 14, 2024 · b45efac · b45efac
1 parent 6f2d01d
commit b45efac
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 7 deletions.
diff --git a/src/Session.php b/src/Session.php
@@ -414,6 +414,8 @@ public static function changeActiveEntities($ID = "all", $is_recursive = false)
                     }
                 }
             } else {
+                $ID = (int)$ID;
+
                /// Check entity validity
                 $ancestors = getAncestorsOf("glpi_entities", $ID);
                 $ok        = false;
@@ -1981,7 +1983,19 @@ public static function getMatchingActiveEntities(/*int|array*/ $entities_ids)/*:
             return [];
         }
 
-        $active_entities_ids = $_SESSION['glpiactiveentities'] ?? [];
+        $active_entities_ids = [];
+        foreach ($_SESSION['glpiactiveentities'] ?? [] as $active_entity_id) {
+            if (!is_int($active_entity_id) && !ctype_digit($active_entity_id)) {
+                // Ensure no unexpected value converted to int
+                // as it would be converted to `0` and would permit access to root entity
+                trigger_error(
+                    sprintf('Unexpected value `%s` found in `$_SESSION[\'glpiactiveentities\']`.', $active_entity_id),
+                    E_USER_WARNING
+                );
+                continue;
+            }
+            $active_entities_ids[] = (int)$active_entity_id;
+        }
 
         if (!is_array($entities_ids) && in_array((int)$entities_ids, $active_entities_ids, true)) {
             return (int)$entities_ids;

diff --git a/tests/functional/Session.php b/tests/functional/Session.php
@@ -632,21 +632,21 @@ public function testGetRightNameForError($module, $right, $expected)
         $this->string(\Session::getRightNameForError($module, $right))->isEqualTo($expected);
     }
 
-    protected function entitiesRestricProvider(): iterable
+    protected function entitiesRestrictProvider(): iterable
     {
         // Special case for -1
         foreach ([-1, "-1", [-1], ["-1"]] as $value) {
             yield [
                 'entity_restrict' => $value,
                 'active_entities' => [0, 1, 2, 3],
-                'result'          => is_array($value) ? [-1] : -1,
+                'result'          => $value,
             ];
         }
 
         // Integer input, matching
         yield [
             'entity_restrict' => 2,
-            'active_entities' => [0, 1, 2, 3],
+            'active_entities' => [0, 1, '2', 3],
             'result'          => 2,
         ];
 
@@ -682,7 +682,7 @@ protected function entitiesRestricProvider(): iterable
         yield [
             'entity_restrict' => [0, '2', 3, 12, 54, 96],
             'active_entities' => [0, 1, 2, 3],
-            'result'          => [0, '2', 3],
+            'result'          => [0, 2, 3],
         ];
 
         // Array input, NONE matching
@@ -712,14 +712,38 @@ protected function entitiesRestricProvider(): iterable
             'active_entities' => [0, 1, 2, 3],
             'result'          => [0, 3],
         ];
+
+        // Active entity may contain a string value
+        // do not know why, but is is the case when only one entity is selected
+        foreach ([2, '2', [2], ['2']] as $entity_restrict) {
+            yield [
+                'entity_restrict' => $entity_restrict,
+                'active_entities' => [0, 1, '2', 3],
+                'result'          => is_array($entity_restrict) ? [2] : 2,
+            ];
+        }
     }
 
     /**
-     * @dataProvider entitiesRestricProvider
+     * @dataProvider entitiesRestrictProvider
      */
     public function testGetMatchingActiveEntities(/*int|array*/ $entity_restrict, ?array $active_entities, /*int|array*/ $result): void
     {
         $_SESSION['glpiactiveentities'] = $active_entities;
-        $this->variable(\Session::getMatchingActiveEntities($entity_restrict))->isEqualTo($result);
+        $this->variable(\Session::getMatchingActiveEntities($entity_restrict))->isIdenticalTo($result);
+    }
+
+    public function testGetMatchingActiveEntitiesWithUnexpectedValue(): void
+    {
+        $_SESSION['glpiactiveentities'] = [0, 1, 2, 'foo', 3];
+
+        $this->when(
+            function () {
+                $this->variable(\Session::getMatchingActiveEntities([2, 3]))->isIdenticalTo([2, 3]);
+            }
+        )->error
+         ->withType(E_USER_WARNING)
+         ->withMessage('Unexpected value `foo` found in `$_SESSION[\'glpiactiveentities\']`.')
+         ->exists();
     }
 }