Force some session settings to ease the server configuration

glpi-project · Nov 27, 2024 · 7830089 · 7830089
1 parent 15145d5
commit 7830089
Show file tree

Hide file tree

Showing 6 changed files with 10 additions and 80 deletions.
diff --git a/src/Auth.php b/src/Auth.php
@@ -1742,7 +1742,6 @@ public static function setRememberMeCookie(string $cookie_value): void
         $cookie_path     = ini_get('session.cookie_path');
         $cookie_domain   = ini_get('session.cookie_domain');
         $cookie_secure   = filter_var(ini_get('session.cookie_secure'), FILTER_VALIDATE_BOOLEAN);
-        $cookie_httponly = filter_var(ini_get('session.cookie_httponly'), FILTER_VALIDATE_BOOLEAN);
         $cookie_samesite = ini_get('session.cookie_samesite');
 
         if (empty($cookie_value) && !isset($_COOKIE[$cookie_name])) {
@@ -1757,7 +1756,7 @@ public static function setRememberMeCookie(string $cookie_value): void
                 'path'     => $cookie_path,
                 'domain'   => $cookie_domain,
                 'secure'   => $cookie_secure,
-                'httponly' => $cookie_httponly,
+                'httponly' => true,
                 'samesite' => $cookie_samesite,
             ]
         );

diff --git a/src/Glpi/Application/SystemConfigurator.php b/src/Glpi/Application/SystemConfigurator.php
@@ -233,8 +233,12 @@ function ($matches) {
     private function setSessionConfiguration(): void
     {
         // Force session to use cookies.
+        ini_set('session.use_trans_sid', '0');
         ini_set('session.use_only_cookies', '1');
 
+        // Force session cookie security.
+        ini_set('session.cookie_httponly', '1');
+
         // Force session cookie name.
         // The cookie name contains the root dir + HTTP host + HTTP port to ensure that it is unique
         // for every GLPI instance, enven if they are served by the same server (mostly for dev envs).

diff --git a/src/Glpi/System/Requirement/SessionsConfiguration.php b/src/Glpi/System/Requirement/SessionsConfiguration.php
@@ -57,22 +57,11 @@ protected function check()
         }
 
        // Check configuration values
-        $is_autostart_on   = ini_get('session.auto_start') == 1;
-        $is_usetranssid_on = ini_get('session.use_trans_sid') == 1
-         || isset($_POST[session_name()]) || isset($_GET[session_name()]);
-
-        if ($is_autostart_on || $is_usetranssid_on) {
-            if ($is_autostart_on && $is_usetranssid_on) {
-                $this->validation_messages[] = __('"session.auto_start" and "session.use_trans_sid" must be set to off.');
-            } else if ($is_autostart_on) {
-                $this->validation_messages[] = __('"session.auto_start" must be set to off.');
-            } else {
-                $this->validation_messages[] = __('"session.use_trans_sid" must be set to off.');
-            }
+        $is_autostart_on = filter_var(ini_get('session.auto_start'), FILTER_VALIDATE_BOOLEAN);
 
+        if ($is_autostart_on) {
+            $this->validation_messages[] = __('"session.auto_start" must be set to off.');
             $this->validated = false;
-            $this->validation_messages[] = __('See .htaccess file in the GLPI root for more information.');
-
             return;
         }
 

diff --git a/src/Glpi/System/Requirement/SessionsSecurityConfiguration.php b/src/Glpi/System/Requirement/SessionsSecurityConfiguration.php
@@ -56,7 +56,6 @@ protected function check()
         $is_cli = isCommandLine();
 
         $cookie_secure   = filter_var(ini_get('session.cookie_secure'), FILTER_VALIDATE_BOOLEAN);
-        $cookie_httponly = filter_var(ini_get('session.cookie_httponly'), FILTER_VALIDATE_BOOLEAN);
         $cookie_samesite = ini_get('session.cookie_samesite');
 
         $is_https_request = ($_SERVER['HTTPS'] ?? 'off') === 'on' || (int)($_SERVER['SERVER_PORT'] ?? null) == 443;
@@ -69,10 +68,6 @@ protected function check()
         if ($is_cli || $cookie_secure_ko) {
             $this->validation_messages[] = __('PHP directive "session.cookie_secure" should be set to "on" when GLPI can be accessed on HTTPS protocol.');
         }
-        $cookie_httponly_ko = !$cookie_httponly;
-        if ($is_cli || $cookie_httponly_ko) {
-            $this->validation_messages[] = __('PHP directive "session.cookie_httponly" should be set to "on" to prevent client-side script to access cookie values.');
-        }
 
         // 'session.cookie_samesite' can be:
         // - 'None':        Cookie will be sent in all cross-origin requests (even POST requests).
@@ -89,7 +84,7 @@ protected function check()
             $this->validation_messages[] = __('PHP directive "session.cookie_samesite" should be set, at least, to "Lax", to prevent cookie to be sent on cross-origin POST requests.');
         }
 
-        $this->validated = !$cookie_secure_ko && !$cookie_httponly_ko && !$cookie_samesite_ko;
+        $this->validated = !$cookie_secure_ko && !$cookie_samesite_ko;
 
         if (!$is_cli && $this->validated) {
             $this->validation_messages[] = __('Sessions configuration is secured.');

diff --git a/tests/functional/Glpi/System/Requirement/SessionsConfiguration.php b/tests/functional/Glpi/System/Requirement/SessionsConfiguration.php
@@ -70,41 +70,6 @@ public function testCheckWithAutostart()
          ->isEqualTo(
              [
                  '"session.auto_start" must be set to off.',
-                 'See .htaccess file in the GLPI root for more information.',
-             ]
-         );
-    }
-
-    public function testCheckWithUseTransId()
-    {
-
-        $this->function->ini_get = function ($name) {
-            return $name == 'session.use_trans_sid' ? '1' : '0';
-        };
-
-        $this->newTestedInstance();
-        $this->boolean($this->testedInstance->isValidated())->isEqualTo(false);
-        $this->array($this->testedInstance->getValidationMessages())
-         ->isEqualTo(
-             [
-                 '"session.use_trans_sid" must be set to off.',
-                 'See .htaccess file in the GLPI root for more information.',
-             ]
-         );
-    }
-
-    public function testCheckWithAutostartAndUseTransId()
-    {
-
-        $this->function->ini_get = '1';
-
-        $this->newTestedInstance();
-        $this->boolean($this->testedInstance->isValidated())->isEqualTo(false);
-        $this->array($this->testedInstance->getValidationMessages())
-         ->isEqualTo(
-             [
-                 '"session.auto_start" and "session.use_trans_sid" must be set to off.',
-                 'See .htaccess file in the GLPI root for more information.',
              ]
          );
     }

diff --git a/tests/functional/Glpi/System/Requirement/SessionsSecurityConfiguration.php b/tests/functional/Glpi/System/Requirement/SessionsSecurityConfiguration.php
@@ -74,7 +74,6 @@ protected function configProvider(): iterable
             // Totally unsecure config
             yield [
                 'cookie_secure'   => $false,
-                'cookie_httponly' => $false,
                 'cookie_samesite' => 'none',
                 'server_https'    => 'on',
                 'server_port'     => '443',
@@ -84,7 +83,6 @@ protected function configProvider(): iterable
             // Strict config
             yield [
                 'cookie_secure'   => $true,
-                'cookie_httponly' => $true,
                 'cookie_samesite' => 'strict',
                 'server_https'    => 'on',
                 'server_port'     => '443',
@@ -94,7 +92,6 @@ protected function configProvider(): iterable
             // cookie_secure can be 0 if query is not on HTTPS
             yield [
                 'cookie_secure'   => $false,
-                'cookie_httponly' => $true,
                 'cookie_samesite' => 'strict',
                 'server_https'    => 'off',
                 'server_port'     => '80',
@@ -104,7 +101,6 @@ protected function configProvider(): iterable
             // cookie_secure should be 1 if query is on HTTPS (detected from $_SERVER['HTTPS'])
             yield [
                 'cookie_secure'   => $false,
-                'cookie_httponly' => $true,
                 'cookie_samesite' => 'strict',
                 'server_https'    => 'on',
                 'server_port'     => null,
@@ -114,23 +110,12 @@ protected function configProvider(): iterable
             // cookie_secure should be 1 if query is on HTTPS (detected from $_SERVER['SERVER_PORT'])
             yield [
                 'cookie_secure'   => $false,
-                'cookie_httponly' => $true,
                 'cookie_samesite' => 'strict',
                 'server_https'    => null,
                 'server_port'     => '443',
                 'is_valid'        => false,
             ];
 
-            // cookie_httponly should be 1
-            yield [
-                'cookie_secure'   => $false,
-                'cookie_httponly' => $false,
-                'cookie_samesite' => 'strict',
-                'server_https'    => 'off',
-                'server_port'     => '80',
-                'is_valid'        => false,
-            ];
-
             // cookie_samesite should be 'Lax', 'Strict', or ''
             $samesite_is_valid = [
                 'None' => false,
@@ -142,15 +127,13 @@ protected function configProvider(): iterable
             foreach ($samesite_is_valid as $samesite => $is_valid) {
                 yield [
                     'cookie_secure'   => $false,
-                    'cookie_httponly' => $true,
                     'cookie_samesite' => $samesite,
                     'server_https'    => 'off',
                     'server_port'     => '80',
                     'is_valid'        => $is_valid,
                 ];
                 yield [
                     'cookie_secure'   => $false,
-                    'cookie_httponly' => $true,
                     'cookie_samesite' => strtolower($samesite),
                     'server_https'    => 'off',
                     'server_port'     => '80',
@@ -165,20 +148,16 @@ protected function configProvider(): iterable
      */
     public function testCheckWithLowercaseLaxSameSiteConfig(
         string $cookie_secure,
-        string $cookie_httponly,
         string $cookie_samesite,
         ?string $server_https,
         ?string $server_port,
         bool $is_valid
     ) {
-        $this->function->ini_get = function ($name) use ($cookie_secure, $cookie_httponly, $cookie_samesite) {
+        $this->function->ini_get = function ($name) use ($cookie_secure, $cookie_samesite) {
             switch ($name) {
                 case 'session.cookie_secure':
                     return $cookie_secure;
                     break;
-                case 'session.cookie_httponly':
-                    return $cookie_httponly;
-                    break;
                 case 'session.cookie_samesite':
                     return $cookie_samesite;
                     break;
@@ -200,7 +179,6 @@ public function testCheckWithLowercaseLaxSameSiteConfig(
                 'Checking the session cookie configuration of the web server cannot be done in the CLI context.',
                 'You should apply the following recommendations for configuring the web server.',
                 'PHP directive "session.cookie_secure" should be set to "on" when GLPI can be accessed on HTTPS protocol.',
-                'PHP directive "session.cookie_httponly" should be set to "on" to prevent client-side script to access cookie values.',
                 'PHP directive "session.cookie_samesite" should be set, at least, to "Lax", to prevent cookie to be sent on cross-origin POST requests.',
             ]
         );