diff --git a/config-templates/config.php b/config-templates/config.php index 87887034d83ba883a89ee4616edb9757d3969c76..d914aac212c596d58ea0c869fa557ce2988d3ad2 100644 --- a/config-templates/config.php +++ b/config-templates/config.php @@ -568,6 +568,18 @@ $config = [ */ 'session.cookie.secure' => false, + /* + * Set the SameSite attribute in the cookie. + * + * You can set this to the strings 'None', 'Lax', or 'Strict' to support + * the RFC6265bis SameSite cookie attribute. If set to null, no SameSite + * attribute will be sent. + * + * Example: + * 'session.cookie.samesite' => 'None', + */ + 'session.cookie.samesite' => null, + /* * Options to override the default settings for php sessions. */ @@ -760,6 +772,7 @@ $config = [ 'language.cookie.secure' => false, 'language.cookie.httponly' => false, 'language.cookie.lifetime' => (60 * 60 * 24 * 900), + 'language.cookie.samesite' => null, /** * Custom getLanguage function called from SimpleSAML\Locale\Language::getLanguage(). diff --git a/lib/SimpleSAML/Locale/Language.php b/lib/SimpleSAML/Locale/Language.php index 31189db618c7febd7aaefd8a4e01ccdc5ed38ae6..05b46776fcd8573306795d8a09ddf2dd34e4b707 100644 --- a/lib/SimpleSAML/Locale/Language.php +++ b/lib/SimpleSAML/Locale/Language.php @@ -424,6 +424,7 @@ class Language 'path' => ($config->getString('language.cookie.path', '/')), 'secure' => ($config->getBoolean('language.cookie.secure', false)), 'httponly' => ($config->getBoolean('language.cookie.httponly', false)), + 'samesite' => ($config->getString('language.cookie.samesite', null)), ]; Utils\HTTP::setCookie($name, $language, $params, false); diff --git a/lib/SimpleSAML/SessionHandler.php b/lib/SimpleSAML/SessionHandler.php index e82df8ae08385c0f5db769d288650a62290b3db0..1b679ce60c5e1d23c885601864faaf993fdd6089 100644 --- a/lib/SimpleSAML/SessionHandler.php +++ b/lib/SimpleSAML/SessionHandler.php @@ -154,6 +154,7 @@ abstract class SessionHandler 'path' => $config->getString('session.cookie.path', '/'), 'domain' => $config->getString('session.cookie.domain', null), 'secure' => $config->getBoolean('session.cookie.secure', false), + 'samesite' => $config->getString('session.cookie.samesite', null), 'httponly' => true, ]; } diff --git a/lib/SimpleSAML/SessionHandlerPHP.php b/lib/SimpleSAML/SessionHandlerPHP.php index da573a2df47f80f5befaf5849272d63ebb3394a0..7d320a2755ef74a2835618454b0c47caf644d8a6 100644 --- a/lib/SimpleSAML/SessionHandlerPHP.php +++ b/lib/SimpleSAML/SessionHandlerPHP.php @@ -76,13 +76,28 @@ class SessionHandlerPHP extends SessionHandler $params = $this->getCookieParams(); if (!headers_sent()) { - session_set_cookie_params( - $params['lifetime'], - $params['path'], - $params['domain'], - $params['secure'], - $params['httponly'] - ); + if (version_compare(PHP_VERSION, '7.3.0', '>=')) { + session_set_cookie_params([ + 'lifetime' => $params['lifetime'], + 'path' => $params['path'], + 'domain' => $params['domain'], + 'secure' => $params['secure'], + 'httponly' => $params['httponly'], + 'samesite' => $params['samesite'], + ]); + } else { + /* in older versions of PHP we need a nasty hack to set RFC6265bis SameSite attribute */ + if ($params['samesite'] !== null and !preg_match('/;\s+samesite/i', $params['path'])) { + $params['path'] .= '; SameSite='.$params['samesite']; + } + session_set_cookie_params( + $params['lifetime'], + $params['path'], + $params['domain'], + $params['secure'], + $params['httponly'] + ); + } } $savepath = $config->getString('session.phpsession.savepath', null); @@ -114,13 +129,17 @@ class SessionHandlerPHP extends SessionHandler session_write_close(); session_name($this->previous_session['name']); - session_set_cookie_params( - $this->previous_session['cookie_params']['lifetime'], - $this->previous_session['cookie_params']['path'], - $this->previous_session['cookie_params']['domain'], - $this->previous_session['cookie_params']['secure'], - $this->previous_session['cookie_params']['httponly'] - ); + if (version_compare(PHP_VERSION, '7.3.0', '>=')) { + session_set_cookie_params($this->previous_session['cookie_params']); + } else { + session_set_cookie_params( + $this->previous_session['cookie_params']['lifetime'], + $this->previous_session['cookie_params']['path'], + $this->previous_session['cookie_params']['domain'], + $this->previous_session['cookie_params']['secure'], + $this->previous_session['cookie_params']['httponly'] + ); + } session_id($this->previous_session['id']); $this->previous_session = []; @session_start(); @@ -333,13 +352,17 @@ class SessionHandlerPHP extends SessionHandler session_write_close(); } - session_set_cookie_params( - $cookieParams['lifetime'], - $cookieParams['path'], - $cookieParams['domain'], - $cookieParams['secure'], - $cookieParams['httponly'] - ); + if (version_compare(PHP_VERSION, '7.3.0', '>=')) { + session_set_cookie_params($cookieParams); + } else { + session_set_cookie_params( + $cookieParams['lifetime'], + $cookieParams['path'], + $cookieParams['domain'], + $cookieParams['secure'], + $cookieParams['httponly'] + ); + } session_id($sessionID); @session_start(); diff --git a/lib/SimpleSAML/Utils/HTTP.php b/lib/SimpleSAML/Utils/HTTP.php index cb0bb41c693d6e12766f3feb129519572364cabd..2ddc64ac584ea03002ea2a355880a06fee6fd9db 100644 --- a/lib/SimpleSAML/Utils/HTTP.php +++ b/lib/SimpleSAML/Utils/HTTP.php @@ -117,7 +117,7 @@ class HTTP // Take care of edge-case where SERVER_PORT is an integer $port = strval($port); - + if ($port !== $default_port) { return ':'.$port; } @@ -490,14 +490,12 @@ class HTTP // data and headers if ($getHeaders) { /** - * Remove for Psalm >=3.0.17 - * @psalm-suppress UndefinedVariable + * @psalm-suppress UndefinedVariable Remove when Psalm >= 3.0.17 */ if (!empty($http_response_header)) { $headers = []; /** - * Remove for Psalm >=3.0.17 - * @psalm-suppress UndefinedVariable + * @psalm-suppress UndefinedVariable Remove when Psalm >= 3.0.17 */ foreach ($http_response_header as $h) { if (preg_match('@^HTTP/1\.[01]\s+\d{3}\s+@', $h)) { @@ -1159,6 +1157,7 @@ class HTTP 'secure' => false, 'httponly' => true, 'raw' => false, + 'samesite' => null, ]; if ($params !== null) { @@ -1181,6 +1180,7 @@ class HTTP if ($value === null) { $expire = time() - 365 * 24 * 60 * 60; + $value = strval($value); } elseif (isset($params['expire'])) { $expire = intval($params['expire']); } elseif ($params['lifetime'] === 0) { @@ -1189,26 +1189,63 @@ class HTTP $expire = time() + intval($params['lifetime']); } - if ($params['raw']) { - $success = @setrawcookie( - $name, - strval($value), - $expire, - $params['path'], - $params['domain'], - $params['secure'], - $params['httponly'] - ); + if (version_compare(PHP_VERSION, '7.3.0', '>=')) { + /* use the new options array for PHP >= 7.3 */ + if ($params['raw']) { + /** @psalm-suppress InvalidArgument Remove when Psalm >= 3.4.10 */ + $success = @setrawcookie( + $name, + $value, + [ + 'expires' => $expire, + 'path' => $params['path'], + 'domain' => $params['domain'], + 'secure' => $params['secure'], + 'httponly' => $params['httponly'], + 'samesite' => $params['samesite'], + ] + ); + } else { + /** @psalm-suppress InvalidArgument Remove when Psalm >= 3.4.10 */ + $success = @setcookie( + $name, + $value, + [ + 'expires' => $expire, + 'path' => $params['path'], + 'domain' => $params['domain'], + 'secure' => $params['secure'], + 'httponly' => $params['httponly'], + 'samesite' => $params['samesite'], + ] + ); + } } else { - $success = @setcookie( - $name, - strval($value), - $expire, - $params['path'], - $params['domain'], - $params['secure'], - $params['httponly'] - ); + /* in older versions of PHP we need a nasty hack to set RFC6265bis SameSite attribute */ + if ($params['samesite'] !== null and !preg_match('/;\s+samesite/i', $params['path'])) { + $params['path'] .= '; SameSite='.$params['samesite']; + } + if ($params['raw']) { + $success = @setrawcookie( + $name, + $value, + $expire, + $params['path'], + $params['domain'], + $params['secure'], + $params['httponly'] + ); + } else { + $success = @setcookie( + $name, + $value, + $expire, + $params['path'], + $params['domain'], + $params['secure'], + $params['httponly'] + ); + } } if (!$success) { diff --git a/tests/lib/SimpleSAML/SessionHandlerPHPTest.php b/tests/lib/SimpleSAML/SessionHandlerPHPTest.php new file mode 100644 index 0000000000000000000000000000000000000000..7e1e720fd2b59af2935c90ff0217952c1e41e7f3 --- /dev/null +++ b/tests/lib/SimpleSAML/SessionHandlerPHPTest.php @@ -0,0 +1,150 @@ +<?php + +namespace SimpleSAML\Test\Utils; + +use PHPUnit\Framework\TestCase; +use SimpleSAML\Test\Utils\ClearStateTestCase; +use SimpleSAML\SessionHandlerPHP; +use SimpleSAML\Configuration; + +class SessionHandlerPHPTest extends ClearStateTestCase +{ + protected $sessionConfig = [ + 'session.cookie.name' => 'SimpleSAMLSessionID', + 'session.cookie.lifetime' => 100, + 'session.cookie.path' => '/ourPath', + 'session.cookie.domain' => 'example.com', + 'session.cookie.secure' => true, + 'session.phpsession.cookiename' => 'SimpleSAML', + ]; + protected $original; + + protected function setUp() + { + $this->original = $_SERVER; + $_SERVER['HTTP_HOST'] = 'example.com'; + $_SERVER['SERVER_NAME'] = 'example.com'; + $_SERVER['HTTPS'] = 'on'; + $_SERVER['SERVER_PORT'] = 443; + $_SERVER['REQUEST_URI'] = '/simplesaml'; + } + + protected function tearDown() + { + $_SERVER = $this->original; + } + + /** + * @covers SimpleSAML\SessionHandlerPHP::__construct() + * @covers SimpleSAML\SessionHandlerPHP::getSessionHandler() + * @covers SimpleSAML\SessionHandler::getSessionHandler() + */ + public function testGetSessionHandler() + { + Configuration::loadFromArray($this->sessionConfig, '[ARRAY]', 'simplesaml'); + $sh = SessionHandlerPHP::getSessionHandler(); + $this->assertInstanceOf('\SimpleSAML\SessionHandlerPHP', $sh); + } + + /** + * @covers SimpleSAML\SessionHandlerPHP::setCookie() + * @runInSeparateProcess + * @requires extension xdebug + */ + public function testSetCookie() + { + Configuration::loadFromArray($this->sessionConfig, '[ARRAY]', 'simplesaml'); + $sh = SessionHandlerPHP::getSessionHandler(); + $sh->setCookie('SimpleSAMLSessionID', 1); + + $headers = xdebug_get_headers(); + $this->assertContains('SimpleSAML=1;', $headers[0]); + $this->assertRegExp('/\b[Ee]xpires=([Mm]on|[Tt]ue|[Ww]ed|[Tt]hu|[Ff]ri|[Ss]at|[Ss]un)/', $headers[0]); + $this->assertRegExp('/\b[Pp]ath=\/ourPath(;|$)/', $headers[0]); + $this->assertRegExp('/\b[Dd]omain=example.com(;|$)/', $headers[0]); + $this->assertRegExp('/\b[Ss]ecure(;|$)/', $headers[0]); + $this->assertRegExp('/\b[Hh]ttp[Oo]nly(;|$)/', $headers[0]); + } + + /** + * @covers SimpleSAML\SessionHandlerPHP::setCookie() + * @runInSeparateProcess + * @requires extension xdebug + */ + public function testSetCookieSameSiteNone() + { + Configuration::loadFromArray(array_merge($this->sessionConfig, ['session.cookie.samesite' => 'None']), '[ARRAY]', 'simplesaml'); + $sh = SessionHandlerPHP::getSessionHandler(); + $sh->setCookie('SimpleSAMLSessionID', 'None'); + + $headers = xdebug_get_headers(); + $this->assertContains('SimpleSAML=None;', $headers[0]); + $this->assertRegExp('/\b[Ss]ame[Ss]ite=None(;|$)/', $headers[0]); + } + + /** + * @covers SimpleSAML\SessionHandlerPHP::setCookie() + * @runInSeparateProcess + * @requires extension xdebug + */ + public function testSetCookieSameSiteLax() + { + Configuration::loadFromArray(array_merge($this->sessionConfig, ['session.cookie.samesite' => 'Lax']), '[ARRAY]', 'simplesaml'); + $sh = SessionHandlerPHP::getSessionHandler(); + $sh->setCookie('SimpleSAMLSessionID', 'Lax'); + + $headers = xdebug_get_headers(); + $this->assertContains('SimpleSAML=Lax;', $headers[0]); + $this->assertRegExp('/\b[Ss]ame[Ss]ite=Lax(;|$)/', $headers[0]); + } + + /** + * @covers SimpleSAML\SessionHandlerPHP::setCookie() + * @runInSeparateProcess + * @requires extension xdebug + */ + public function testSetCookieSameSiteStrict() + { + Configuration::loadFromArray(array_merge($this->sessionConfig, ['session.cookie.samesite' => 'Strict']), '[ARRAY]', 'simplesaml'); + $sh = SessionHandlerPHP::getSessionHandler(); + $sh->setCookie('SimpleSAMLSessionID', 'Strict'); + + $headers = xdebug_get_headers(); + $this->assertContains('SimpleSAML=Strict;', $headers[0]); + $this->assertRegExp('/\b[Ss]ame[Ss]ite=Strict(;|$)/', $headers[0]); + } + + /** + * @covers SimpleSAML\SessionHandlerPHP::restorePrevious() + * @runInSeparateProcess + * @requires extension xdebug + */ + public function testRestorePrevious() + { + session_name('PHPSESSID'); + $sid = session_id(); + session_start(); + + Configuration::loadFromArray($this->sessionConfig, '[ARRAY]', 'simplesaml'); + $sh = SessionHandlerPHP::getSessionHandler(); + $sh->setCookie('SimpleSAMLSessionID', 'Restore'); + $oldSh = $sh->restorePrevious(); + + $headers = xdebug_get_headers(); + $this->assertContains('PHPSESSID='.$sid, $headers[0]); + $this->assertContains('SimpleSAML=Restore;', $headers[1]); + $this->assertContains('PHPSESSID='.$sid, $headers[2]); + $this->assertEquals($headers[0], $headers[2]); + } + + /** + * @covers SimpleSAML\SessionHandlerPHP::newSessionId() + */ + public function testNewSessionId() + { + Configuration::loadFromArray($this->sessionConfig, '[ARRAY]', 'simplesaml'); + $sh = SessionHandlerPHP::getSessionHandler(); + $sid = $sh->newSessionId(); + $this->assertStringMatchesFormat('%s', $sid); + } +} diff --git a/tests/lib/SimpleSAML/Utils/HTTPTest.php b/tests/lib/SimpleSAML/Utils/HTTPTest.php index 0940abc17d534ab35f11f37298482e5ab505621b..b5a1a4f18b5046d1d54c63dfc561fe7e42d2f5a4 100644 --- a/tests/lib/SimpleSAML/Utils/HTTPTest.php +++ b/tests/lib/SimpleSAML/Utils/HTTPTest.php @@ -3,10 +3,11 @@ namespace SimpleSAML\Test\Utils; use PHPUnit\Framework\TestCase; +use SimpleSAML\Test\Utils\ClearStateTestCase; use SimpleSAML\Utils\HTTP; use SimpleSAML\Configuration; -class HTTPTest extends TestCase +class HTTPTest extends ClearStateTestCase { /** * Set up the environment ($_SERVER) populating the typical variables from a given URL. @@ -451,4 +452,77 @@ class HTTPTest extends TestCase $this->assertEquals(HTTP::getFirstPathElement(false), 'test'); $_SERVER = $original; } + + /** + * @covers SimpleSAML\Utils\HTTP::setCookie() + * @runInSeparateProcess + * @requires extension xdebug + */ + public function testSetCookie() + { + $original = $_SERVER; + Configuration::loadFromArray([ + 'baseurlpath' => 'https://example.com/simplesaml/', + ], '[ARRAY]', 'simplesaml'); + $url = 'https://example.com/a?b=c'; + $this->setupEnvFromURL($url); + + HTTP::setCookie('TestCookie', 'value%20', ['expire'=> 2147483640, 'path'=>'/ourPath', 'domain'=>'example.com', 'secure'=>true, 'httponly'=>true]); + HTTP::setCookie('RawCookie', 'value%20', ['lifetime'=>100, 'path'=>'/ourPath', 'domain'=>'example.com', 'secure'=>true, 'httponly'=>true, 'raw'=>true]); + + $headers = xdebug_get_headers(); + $this->assertContains('TestCookie=value%2520;', $headers[0]); + $this->assertRegExp('/\b[Ee]xpires=[Tt]ue/', $headers[0]); + $this->assertRegExp('/\b[Pp]ath=\/ourPath(;|$)/', $headers[0]); + $this->assertRegExp('/\b[Dd]omain=example.com(;|$)/', $headers[0]); + $this->assertRegExp('/\b[Ss]ecure(;|$)/', $headers[0]); + $this->assertRegExp('/\b[Hh]ttp[Oo]nly(;|$)/', $headers[0]); + + $this->assertContains('RawCookie=value%20;', $headers[1]); + $this->assertRegExp('/\b[Ee]xpires=([Mm]on|[Tt]ue|[Ww]ed|[Tt]hu|[Ff]ri|[Ss]at|[Ss]un)/', $headers[1]); + $this->assertRegExp('/\b[Pp]ath=\/ourPath(;|$)/', $headers[1]); + $this->assertRegExp('/\b[Dd]omain=example.com(;|$)/', $headers[1]); + $this->assertRegExp('/\b[Ss]ecure(;|$)/', $headers[1]); + $this->assertRegExp('/\b[Hh]ttp[Oo]nly(;|$)/', $headers[1]); + + $_SERVER = $original; + } + + /** + * @covers SimpleSAML\Utils\HTTP::setCookie() + */ + public function testSetCookieInsecure() + { + $this->expectException(\SimpleSAML\Error\CannotSetCookie::class); + + $original = $_SERVER; + Configuration::loadFromArray([ + 'baseurlpath' => 'http://example.com/simplesaml/', + ], '[ARRAY]', 'simplesaml'); + $url = 'http://example.com/a?b=c'; + $this->setupEnvFromURL($url); + + HTTP::setCookie('testCookie', 'value', ['secure' => true], true); + + $_SERVER = $original; + } + + /** + * @covers SimpleSAML\Utils\HTTP::setCookie() + * @runInSeparateProcess + * @requires extension xdebug + */ + public function testSetCookieSameSite() + { + HTTP::setCookie('SSNull', 'value', ['samesite' => null]); + HTTP::setCookie('SSNone', 'value', ['samesite' => 'None']); + HTTP::setCookie('SSLax', 'value', ['samesite' => 'Lax']); + HTTP::setCookie('SSStrict', 'value', ['samesite' => 'Strict']); + + $headers = xdebug_get_headers(); + $this->assertNotRegExp('/\b[Ss]ame[Ss]ite=/', $headers[0]); + $this->assertRegExp('/\b[Ss]ame[Ss]ite=None(;|$)/', $headers[1]); + $this->assertRegExp('/\b[Ss]ame[Ss]ite=Lax(;|$)/', $headers[2]); + $this->assertRegExp('/\b[Ss]ame[Ss]ite=Strict(;|$)/', $headers[3]); + } }