Skip to content

Commit

Permalink
Fix: reload session entities in case of modification (#17819)
Browse files Browse the repository at this point in the history
* Fix: reload session entities in case of modification

* cedric

* shouldReloadActiveEntities

* Apply suggestions from code review

Co-authored-by: Cédric Anne <cedric.anne@gmail.com>

* array_key_exists

---------

Co-authored-by: Cédric Anne <cedric.anne@gmail.com>
  • Loading branch information
Rom1-B and cedric-anne authored Sep 13, 2024
1 parent ce5a5dd commit a7d2e12
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 0 deletions.
5 changes: 5 additions & 0 deletions inc/includes.php
Original file line number Diff line number Diff line change
Expand Up @@ -181,4 +181,9 @@
// Manage entity change
if (isset($_REQUEST["force_entity"]) && ($_SESSION["glpiactive_entity"] ?? -1) != $_REQUEST["force_entity"]) {
Session::changeActiveEntities($_REQUEST["force_entity"], true);
} elseif (Session::shouldReloadActiveEntities()) {
Session::changeActiveEntities(
$_SESSION["glpiactive_entity"],
$_SESSION["glpiactive_entity_recursive"]
);
}
28 changes: 28 additions & 0 deletions src/Session.php
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,33 @@ public static function initNavigateListItems($itemtype, $title = "", $url = null
}


/**
* Check if active entities should be reloaded.
*
* @return bool true if active entities should be reloaded, false otherwise
*/
public static function shouldReloadActiveEntities(): bool
{
if (!array_key_exists('glpiactive_entity', $_SESSION)) {
return false;
}
$glpiactiveentities = $_SESSION['glpiactiveentities'] ?? [];
if (count($glpiactiveentities)) {
$glpiactive_entity = $_SESSION['glpiactive_entity'];
$glpiactive_entity_recursive = $_SESSION['glpiactive_entity_recursive'] ?? false;
$entities = [$glpiactive_entity];
if ($glpiactive_entity_recursive) {
$entities = getSonsOf("glpi_entities", $glpiactive_entity);
}

return count($entities) !== count($glpiactiveentities)
|| array_diff($entities, $glpiactiveentities) !== []
|| array_diff($glpiactiveentities, $entities) !== [];
}
return false;
}


/**
* Change active entity to the $ID one. Update glpiactiveentities session variable.
* Reload groups related to this entity.
Expand Down Expand Up @@ -415,6 +442,7 @@ public static function changeActiveEntities($ID = "all", $is_recursive = false)
}
}
}
$is_recursive = true;
} else {
$ID = (int)$ID;

Expand Down
56 changes: 56 additions & 0 deletions tests/functional/Session.php
Original file line number Diff line number Diff line change
Expand Up @@ -797,4 +797,60 @@ function () {
->withMessage('Unexpected value `null` found in `$_SESSION[\'glpiactiveentities\']`.')
->exists();
}

public function testShouldReloadActiveEntities(): void
{
$this->login('glpi', 'glpi');

$ent0 = getItemByTypeName('Entity', '_test_root_entity', true);
$ent1 = getItemByTypeName('Entity', '_test_child_1', true);
$ent2 = getItemByTypeName('Entity', '_test_child_2', true);

// Create a new entity
$entity_id = $this->createItem(\Entity::class, [
'name' => __METHOD__,
'entities_id' => $ent1
])->getID();

$this->boolean(\Session::changeActiveEntities($ent1, true))->isTrue();

// The entity goes out of scope -> reloaded TRUE
$this->updateItem(\Entity::class, $entity_id, [
'entities_id' => $ent0
]);
$this->boolean(\Session::shouldReloadActiveEntities())->isTrue();

$this->boolean(\Session::changeActiveEntities($ent2, true))->isTrue();

// The entity enters the scope -> reloaded TRUE
$this->updateItem(\Entity::class, $entity_id, [
'entities_id' => $ent2
]);
$this->boolean(\Session::shouldReloadActiveEntities())->isTrue();

$this->boolean(\Session::changeActiveEntities($ent1, true))->isTrue();

// The entity remains out of scope -> reloaded FALSE
$this->updateItem(\Entity::class, $entity_id, [
'entities_id' => $ent0
]);
$this->boolean(\Session::shouldReloadActiveEntities())->isFalse();

$this->boolean(\Session::changeActiveEntities($ent1, false))->isTrue();

// The entity remains out of scope -> reloaded FALSE
$this->updateItem(\Entity::class, $entity_id, [
'entities_id' => $ent1
]);
$this->boolean(\Session::shouldReloadActiveEntities())->isFalse();

// See all entities -> reloaded FALSE
$this->boolean(\Session::changeActiveEntities('all'))->isTrue();

$this->updateItem(\Entity::class, $entity_id, [
'entities_id' => $ent2
]);

$this->boolean(\Session::shouldReloadActiveEntities())->isFalse();
}
}

1 comment on commit a7d2e12

@chutuananh2k
Copy link

@chutuananh2k chutuananh2k commented on a7d2e12 Dec 27, 2024

Choose a reason for hiding this comment

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

@Rom1-B @cedric-anne This fix won't stop CVE-2024-50339 (Unauthenticated session hijacking).

Please sign in to comment.