Skip to content

Commit

Permalink
Fix session configuration directives handling
Browse files Browse the repository at this point in the history 
fixes #17312
  • Loading branch information
@cedric-anne
cedric-anne committed Jun 17, 2024
1 parent d4a545d commit e85e580
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 77 deletions.
4 changes: 2 additions & 2 deletions src/Auth.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1914,8 +1914,8 @@ public static function setRememberMeCookie(string $cookie_value): void
$cookie_lifetime = empty($cookie_value) ? time() - 3600 : time() + $CFG_GLPI['login_remember_time'];
$cookie_path = ini_get('session.cookie_path');
$cookie_domain = ini_get('session.cookie_domain');
$cookie_secure = (bool)ini_get('session.cookie_secure');
$cookie_httponly = (bool)ini_get('session.cookie_httponly');
$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])) {
Expand Down
4 changes: 2 additions & 2 deletions src/System/Requirement/SessionsSecurityConfiguration.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ protected function check()
{
$is_cli = isCommandLine();

$cookie_secure = (bool)ini_get('session.cookie_secure');
$cookie_httponly = (bool)ini_get('session.cookie_httponly');
$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;
Expand Down
179 changes: 106 additions & 73 deletions tests/functional/Glpi/System/Requirement/SessionsSecurityConfiguration.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,91 +39,124 @@ class SessionsSecurityConfiguration extends \GLPITestCase
{
protected function configProvider(): iterable
{
// Totally unsecure config
yield [
'cookie_secure' => '0',
'cookie_httponly' => '0',
'cookie_samesite' => 'none',
'server_https' => 'on',
'server_port' => '443',
'is_valid' => false,
];

// Strict config
yield [
'cookie_secure' => '1',
'cookie_httponly' => '1',
'cookie_samesite' => 'strict',
'server_https' => 'on',
'server_port' => '443',
'is_valid' => true,
];

// cookie_secure can be 0 if query is not on HTTPS
yield [
'cookie_secure' => '0',
'cookie_httponly' => '1',
'cookie_samesite' => 'strict',
'server_https' => 'off',
'server_port' => '80',
'is_valid' => true,
];

// cookie_secure should be 1 if query is on HTTPS (detected from $_SERVER['HTTPS'])
yield [
'cookie_secure' => '0',
'cookie_httponly' => '1',
'cookie_samesite' => 'strict',
'server_https' => 'on',
'server_port' => null,
'is_valid' => false,
// Boolean values are supposed to be converted by PHP internal logic when the PHP ini files are parsed,
// but they will not be converted to boolean when they are programatically set by a `ini_set()` call.
$boolean_specs = [
[
'true' => true,
'false' => false,
],
[
'true' => '1',
'false' => '0',
],
[
'true' => 'true',
'false' => 'false',
],
[
'true' => 'on',
'false' => 'off',
],
[
'true' => 'yes',
'false' => 'no',
],
[
'true' => '1',
'false' => 'none',
],
];
foreach ($boolean_specs as $specs) {
$true = $specs['true'];
$false = $specs['false'];

// cookie_secure should be 1 if query is on HTTPS (detected from $_SERVER['SERVER_PORT'])
yield [
'cookie_secure' => '0',
'cookie_httponly' => '1',
'cookie_samesite' => 'strict',
'server_https' => null,
'server_port' => '443',
'is_valid' => false,
];
// Totally unsecure config
yield [
'cookie_secure' => $false,
'cookie_httponly' => $false,
'cookie_samesite' => 'none',
'server_https' => 'on',
'server_port' => '443',
'is_valid' => false,
];

// cookie_httponly should be 1
yield [
'cookie_secure' => '0',
'cookie_httponly' => '0',
'cookie_samesite' => 'strict',
'server_https' => 'off',
'server_port' => '80',
'is_valid' => false,
];
// Strict config
yield [
'cookie_secure' => $true,
'cookie_httponly' => $true,
'cookie_samesite' => 'strict',
'server_https' => 'on',
'server_port' => '443',
'is_valid' => true,
];

// cookie_samesite should be 'Lax', 'Strict', or ''
$samesite_is_valid = [
'None' => false,
'Lax' => true,
'Strict' => true,
'' => true,
'NotAnExpectedValue' => false,
];
foreach ($samesite_is_valid as $samesite => $is_valid) {
// cookie_secure can be 0 if query is not on HTTPS
yield [
'cookie_secure' => '0',
'cookie_httponly' => '1',
'cookie_samesite' => $samesite,
'cookie_secure' => $false,
'cookie_httponly' => $true,
'cookie_samesite' => 'strict',
'server_https' => 'off',
'server_port' => '80',
'is_valid' => $is_valid,
'is_valid' => true,
];

// 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,
'is_valid' => false,
];

// 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' => '0',
'cookie_httponly' => '1',
'cookie_samesite' => strtolower($samesite),
'cookie_secure' => $false,
'cookie_httponly' => $false,
'cookie_samesite' => 'strict',
'server_https' => 'off',
'server_port' => '80',
'is_valid' => $is_valid,
'is_valid' => false,
];

// cookie_samesite should be 'Lax', 'Strict', or ''
$samesite_is_valid = [
'None' => false,
'Lax' => true,
'Strict' => true,
'' => true,
'NotAnExpectedValue' => false,
];
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',
'is_valid' => $is_valid,
];
}
}
}

Expand Down

0 comments on commit e85e580

Please sign in to comment.