Fix session initialization

Author: cedric-anne

CHANGELOG.md

@@ -544,6 +544,7 @@ The present file will list all changes made to the project; according to the
 - `Toolbox::sodiumEncrypt()`
 - `Toolbox::unclean_cross_side_scripting_deep()`
 - `Transfer::manageConnectionComputer()`
+- `Update::initSession()`
 - `User::getDelegateGroupsForUser()`
 - `User::showDebug()`
 - `User::title()`

install/install.php

@@ -488,13 +488,6 @@ function update1($dbname)
 
 //------------Start of install script---------------------------
 
-
-// Use default session dir if not writable
-if (is_writable(GLPI_SESSION_DIR)) {
-    Session::setPath();
-}
-
-Session::start();
 error_reporting(0); // we want to check system before affraid the user.
 
 

install/update.php

@@ -62,7 +62,6 @@
 Config::loadLegacyConfiguration();
 
 $update = new Update($DB);
-$update->initSession();
 
 if (isset($_POST['update_end'])) {
     if (isset($_POST['send_stats'])) {

src/Glpi/Config/LegacyConfigurators/SessionStart.php

@@ -34,56 +34,58 @@
 
 namespace Glpi\Config\LegacyConfigurators;
 
-use Glpi\Config\ConfigProviderHasRequestTrait;
 use Session;
-use Glpi\Config\ConfigProviderWithRequestInterface;
 use Glpi\Config\LegacyConfigProviderInterface;
+use Symfony\Component\HttpFoundation\Request;
 
-final class SessionStart implements LegacyConfigProviderInterface, ConfigProviderWithRequestInterface
+final class SessionStart implements LegacyConfigProviderInterface
 {
-    use ConfigProviderHasRequestTrait;
-
-    /**
-     * An array of regular expressions of the paths that disable the Session.
-     */
-    private const NO_COOKIE_PATHS = [
-        '/api(rest)?\.php.*',
-        '/caldav\.php.*',
-        '/front/cron\.php.*',
-    ];
-
-    private const NO_SESSION_PATHS = [
-        '/api(rest)?\.php.*',
-    ];
-
     public function execute(): void
     {
-        $path = $this->getRequest()->getRequestUri();
-        $path = '/' . ltrim($path, '/');
+        // The session must be started even in CLI context.
+        // The GLPI code refers to the session in many places
+        // and we cannot safely remove its initialization in the CLI context.
+        $start_session = true;
 
-        $noCookiePaths = '~^' . implode('|', \array_map(static fn ($regex) => '(?:' . $regex . ')', self::NO_COOKIE_PATHS)) . '$~sUu';
+        if (isset($_SERVER['REQUEST_URI'])) {
+            // Specific configuration related to web context
 
-        Session::setPath();
+            $request = Request::createFromGlobals();
+            $path = $request->getPathInfo();
 
-        if (
-            \preg_match($noCookiePaths, $path)
-            || (\preg_match('~^/front/planning\.php~Uu', $path) && $this->getRequest()->query->has('genical'))
-        ) {
-            // Disable session cookie for these paths
-            ini_set('session.use_cookies', 0);
-        }
+            $use_cookies = true;
+            if (\str_starts_with($path, '/api.php') || \str_starts_with($path, '/apirest.php')) {
+                // API clients must not use cookies, as the session token is expected to be passed in headers.
+                $use_cookies = false;
+                // The API endpoint is strating the session manually.
+                $start_session = false;
+            } elseif (\str_starts_with($path, '/caldav.php')) {
+                // CalDAV clients must not use cookies, as the authentication is expected to be passed in headers.
+                $use_cookies = false;
+            } elseif (\str_starts_with($path, '/front/cron.php')) {
+                // The cron endpoint is not expected to use the authenticated user session.
+                $use_cookies = false;
+            } elseif (\str_starts_with($path, '/front/planning.php') && $request->query->has('genical')) {
+                // The `genical` endpoint must not use cookies, as the authentication is expected to be passed in the query parameters.
+                $use_cookies = false;
+            }
 
-        $noSessionPaths = '~^' . implode('|', \array_map(static fn ($regex) => '(?:' . $regex . ')', self::NO_SESSION_PATHS)) . '$~sUu';
-        if (
-            !\preg_match($noSessionPaths, $path)
-        ) {
-            // Disable session cookie for these paths
-            Session::start();
+            if (!$use_cookies) {
+                ini_set('session.use_cookies', 0);
+            }
         }
 
-        // Default Use mode
-        if (!isset($_SESSION['glpi_use_mode'])) {
-            $_SESSION['glpi_use_mode'] = Session::NORMAL_MODE;
+        if ($start_session) {
+            if (Session::canWriteSessionFiles()) {
+                Session::setPath();
+            } else {
+                \trigger_error(
+                    sprintf('Unable to write session files on `%s`.', GLPI_SESSION_DIR),
+                    E_USER_WARNING
+                );
+            }
+
+            Session::start();
         }
     }
 }

src/Glpi/Controller/IndexController.php

@@ -96,9 +96,6 @@ private function call(): void
             if (file_exists(GLPI_ROOT . '/install/install.php')) {
                 Html::redirect("install/install.php");
             } else {
-                // Init session (required by header display logic)
-                Session::setPath();
-                Session::start();
                 Session::loadLanguage('', false);
                 // Prevent inclusion of debug information in footer, as they are based on vars that are not initialized here.
                 $_SESSION['glpi_use_mode'] = Session::NORMAL_MODE;

src/Glpi/Debug/Profile.php

@@ -217,7 +217,7 @@ public function getDebugInfo(): array
 
     public function save(): void
     {
-        if ($_SESSION['glpi_use_mode'] !== \Session::DEBUG_MODE) {
+        if (($_SESSION['glpi_use_mode'] ?? null) !== \Session::DEBUG_MODE) {
             // Don't save debug info for non-debug requests
             return;
         }

src/Session.php

@@ -257,6 +257,11 @@ public static function start()
 
         // Define current time for sync of action timing
         $_SESSION["glpi_currenttime"] = date("Y-m-d H:i:s");
+
+        // Define session default mode
+        if (!isset($_SESSION['glpi_use_mode'])) {
+            $_SESSION['glpi_use_mode'] = Session::NORMAL_MODE;
+        }
     }
 
 

src/Update.php

@@ -34,7 +34,6 @@
  */
 
 use Glpi\Helpdesk\DefaultDataManager;
-use Glpi\Form\Form;
 use Glpi\Rules\RulesManager;
 use Glpi\System\Diagnostic\DatabaseSchemaIntegrityChecker;
 use Glpi\Toolbox\VersionParser;
@@ -44,7 +43,6 @@
  **/
 class Update
 {
-    private $args = [];
     private $DB;
     /**
      * @var Migration
@@ -65,35 +63,14 @@ class Update
      * Constructor
      *
      * @param object $DB   Database instance
-     * @param array  $args Command line arguments; default to empty array
      * @param string $migrations_directory
      */
-    public function __construct($DB, $args = [], string $migrations_directory = GLPI_ROOT . '/install/migrations/')
+    public function __construct($DB, string $migrations_directory = GLPI_ROOT . '/install/migrations/')
     {
         $this->DB = $DB;
-        $this->args = $args;
         $this->migrations_directory = $migrations_directory;
     }
 
-    /**
-     * Initialize session for update
-     *
-     * @return void
-     */
-    public function initSession()
-    {
-        if (Session::canWriteSessionFiles()) {
-            Session::setPath();
-        }
-        Session::start();
-
-        if (isCommandLine()) {
-           // Init debug variable
-            $_SESSION = ['glpilanguage' => (isset($this->args['lang']) ? $this->args['lang'] : 'en_GB')];
-            $_SESSION["glpi_currenttime"] = date("Y-m-d H:i:s");
-        }
-    }
-
     /**
      * Get current values (versions, lang, ...)
      *

tests/functional/Update.php

@@ -54,26 +54,6 @@ public function testCurrents()
         $this->array($update->getCurrents())->isEqualTo($expected);
     }
 
-    public function testInitSession()
-    {
-        global $DB;
-
-        $update = new \Update($DB);
-        session_destroy();
-        $this->variable(session_status())->isIdenticalTo(PHP_SESSION_NONE);
-
-        $update->initSession();
-        $this->variable(session_status())->isIdenticalTo(PHP_SESSION_ACTIVE);
-
-        $this->array($_SESSION)->hasKeys([
-            'glpilanguage',
-            'glpi_currenttime',
-        ])->notHasKeys([
-            'glpi_use_mode',
-            'use_log_in_files'
-        ]);
-    }
-
     public function testSetMigration()
     {
         global $DB;
@@ -401,7 +381,7 @@ public function testGetMigrationsToDo(string $current_version, bool $force_lates
         $method->setAccessible(true);
 
         global $DB;
-        $update = new \Update($DB, [], vfsStream::url('install/migrations'));
+        $update = new \Update($DB, vfsStream::url('install/migrations'));
         $this->array($method->invokeArgs($update, [$current_version, $force_latest]))->isIdenticalTo($expected_migrations);
     }
 }