diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index c978f91..2c0f654 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -33,7 +33,7 @@ jobs: ports: - 6379:6379 - name: PHP 8.1 + name: PHP 8.3 steps: - name: Checkout @@ -42,7 +42,7 @@ jobs: - name: Setup PHP uses: shivammathur/setup-php@v2 with: - php-version: 8.1 + php-version: 8.3 tools: composer coverage: xdebug diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 12205a4..0ff2d72 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,5 +1,5 @@ --- -image: registry.gitlab.com/aplus-framework/images/base:2 +image: registry.gitlab.com/aplus-framework/images/base:4 include: - template: Security/SAST.gitlab-ci.yml diff --git a/composer.json b/composer.json index 11f9b82..7ca4c65 100644 --- a/composer.json +++ b/composer.json @@ -33,16 +33,16 @@ } ], "require": { - "php": ">=8.1", + "php": ">=8.3", "ext-memcached": "*", "ext-redis": "*", - "aplus/database": "^3.0", - "aplus/debug": "^3.1", - "aplus/log": "^3.0" + "aplus/database": "^4.0", + "aplus/debug": "^4.0", + "aplus/log": "^4.0" }, "require-dev": { "ext-xdebug": "*", - "aplus/coding-standard": "^2.0", + "aplus/coding-standard": "^2.8", "ergebnis/composer-normalize": "^2.25", "jetbrains/phpstorm-attributes": "^1.0", "phpmd/phpmd": "^2.13", diff --git a/docker-compose.yml b/docker-compose.yml index 8540a79..5557e35 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,7 +1,7 @@ version: "3" services: package: - image: registry.gitlab.com/aplus-framework/images/package:2 + image: registry.gitlab.com/aplus-framework/images/package:4 container_name: package-session working_dir: /package volumes: diff --git a/guide/index.rst b/guide/index.rst index 81eff2d..258402c 100644 --- a/guide/index.rst +++ b/guide/index.rst @@ -76,6 +76,14 @@ or by the ``start`` method: 'name' => 'session_id' ]); // bool +Custom Options +^^^^^^^^^^^^^^ + +Custom options only work if they are passed through the Session constructor. + +- `Auto Regenerate ID`_ +- `Set-Cookie Permanent`_ + Auto Regenerate ID ################## @@ -92,6 +100,24 @@ options: 'auto_regenerate_destroy' => true, ])); +This will help avoid +`Session Fixation `_. + +Set-Cookie Permanent +#################### + +It is possible to send the session's Set-Cookie header in all HTTP responses by +setting the ``set_cookie_permanent`` option: + +.. code-block:: php + + $session = new Session([ + 'set_cookie_permanent' => true, + ])); + +This will cause the session cookie expiration date to be updated in the browser +on every response. + Managing Data ------------- @@ -291,6 +317,25 @@ the example below: $saveHandler = new DatabaseHandler(); $saveHandler->setDatabase($database); // static +Database Table +############## + +A basic example of a table for sessions is below: + +.. code-block:: sql + + CREATE TABLE `Sessions` ( + `id` varchar(128) NOT NULL, + `timestamp` timestamp NOT NULL, + `data` blob NOT NULL, + `ip` varchar(45) NOT NULL, -- optional + `ua` varchar(255) NOT NULL, -- optional + PRIMARY KEY (`id`), + KEY `timestamp` (`timestamp`), + KEY `ip` (`ip`), -- optional + KEY `ua` (`ua`) -- optional + ); + Files Handler ^^^^^^^^^^^^^ diff --git a/src/Debug/SessionCollection.php b/src/Debug/SessionCollection.php new file mode 100644 index 0000000..6e5070a --- /dev/null +++ b/src/Debug/SessionCollection.php @@ -0,0 +1,22 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ +namespace Framework\Session\Debug; + +use Framework\Debug\Collection; + +/** + * Class SessionCollection. + * + * @package session + */ +class SessionCollection extends Collection +{ + protected string $iconPath = __DIR__ . '/icons/session.svg'; +} diff --git a/src/Debug/SessionCollector.php b/src/Debug/SessionCollector.php index dd00396..1b10f50 100644 --- a/src/Debug/SessionCollector.php +++ b/src/Debug/SessionCollector.php @@ -10,7 +10,6 @@ namespace Framework\Session\Debug; use Framework\Debug\Collector; -use Framework\Debug\Debugger; use Framework\Helpers\ArraySimple; use Framework\Session\SaveHandler; use Framework\Session\SaveHandlers\DatabaseHandler; @@ -96,7 +95,7 @@ protected function renderData() : string Key - Value + Value Type @@ -104,7 +103,7 @@ protected function renderData() : string
@@ -138,7 +137,7 @@ protected function renderFlashOld() : string Key - Value + Value Type @@ -146,7 +145,7 @@ protected function renderFlashOld() : string
@@ -170,7 +169,7 @@ protected function renderFlashNew() : string Key - Value + Value Type @@ -178,7 +177,7 @@ protected function renderFlashNew() : string
@@ -200,7 +199,7 @@ protected function renderTemp() : string Key - Value + Value Type TTL @@ -209,7 +208,7 @@ protected function renderTemp() : string
@@ -315,6 +314,10 @@ protected function renderCookieParams() : string +

+ Set-Cookie: options['set_cookie_permanent'] ? 'Permanent' : 'Normal' ?> +

diff --git a/src/SaveHandler.php b/src/SaveHandler.php index 5422856..ed6a762 100644 --- a/src/SaveHandler.php +++ b/src/SaveHandler.php @@ -41,7 +41,7 @@ abstract class SaveHandler implements \SessionHandlerInterface, \SessionUpdateTi * * @var false|string */ - protected string | false $lockId = false; + protected false | string $lockId = false; /** * Tells if the session exists (if was read). * @@ -60,6 +60,16 @@ abstract class SaveHandler implements \SessionHandlerInterface, \SessionUpdateTi * @var Logger|null */ protected ?Logger $logger; + /** + * It says that the handler object was set with an externally created object. + * + * If this is true, the handler object should not be changed or removed (in the close method). + * + * Some functions, such as `session_regenerate_id`, call the close method and then open again! + * + * @var bool + */ + protected bool $setByExternal = false; /** * SessionSaveHandler constructor. @@ -70,7 +80,7 @@ abstract class SaveHandler implements \SessionHandlerInterface, \SessionUpdateTi public function __construct( #[SensitiveParameter] array $config = [], - Logger $logger = null + ?Logger $logger = null ) { $this->prepareConfig($config); $this->logger = $logger; @@ -195,11 +205,15 @@ protected function getKeySuffix() : string * @param string $id The session id * * @see https://www.php.net/manual/en/sessionupdatetimestamphandlerinterface.validateid.php + * @see https://wiki.php.net/rfc/deprecations_php_8_4#sessionsid_length_and_sessionsid_bits_per_character * * @return bool Returns TRUE if the id is valid, otherwise FALSE */ public function validateId($id) : bool { + if (\PHP_VERSION_ID >= 80400) { + return (bool) \preg_match('#\A[0-9a-f]{32}\z#', $id); + } $bits = \ini_get('session.sid_bits_per_character') ?: 5; $length = \ini_get('session.sid_length') ?: 40; $bitsRegex = [ @@ -298,7 +312,7 @@ abstract public function destroy($id) : bool; * @return false|int Returns the number of deleted session data for success, * false for failure */ - abstract public function gc($max_lifetime) : int | false; + abstract public function gc($max_lifetime) : false | int; /** * Acquire a lock for a session id. diff --git a/src/SaveHandlers/DatabaseHandler.php b/src/SaveHandlers/DatabaseHandler.php index 2c1bd37..2492370 100644 --- a/src/SaveHandlers/DatabaseHandler.php +++ b/src/SaveHandlers/DatabaseHandler.php @@ -35,6 +35,8 @@ * ); * ``` * + * NOTE: As of PHP 8.4 the id column can be `char(32)`. + * * @package session */ class DatabaseHandler extends SaveHandler @@ -98,6 +100,7 @@ protected function prepareConfig(#[SensitiveParameter] array $config) : void public function setDatabase(Database $database) : static { + $this->setByExternal = true; $this->database = $database; return $this; } @@ -278,7 +281,9 @@ public function updateTimestamp($id, $data) : bool public function close() : bool { $closed = !($this->lockId && !$this->unlock()); - $this->database = null; + if ($this->setByExternal === false) { + $this->database = null; + } return $closed; } @@ -299,7 +304,7 @@ public function destroy($id) : bool return true; } - public function gc($max_lifetime) : int | false + public function gc($max_lifetime) : false | int { try { $this->database ??= new Database($this->config); diff --git a/src/SaveHandlers/FilesHandler.php b/src/SaveHandlers/FilesHandler.php index 092eadb..f9fe8c5 100644 --- a/src/SaveHandlers/FilesHandler.php +++ b/src/SaveHandlers/FilesHandler.php @@ -187,7 +187,7 @@ public function destroy($id) : bool return !\is_file($filename) || \unlink($filename); } - public function gc($max_lifetime) : int | false + public function gc($max_lifetime) : false | int { $dirHandle = \opendir($this->config['directory']); if ($dirHandle === false) { diff --git a/src/SaveHandlers/MemcachedHandler.php b/src/SaveHandlers/MemcachedHandler.php index 948587d..08444b1 100644 --- a/src/SaveHandlers/MemcachedHandler.php +++ b/src/SaveHandlers/MemcachedHandler.php @@ -94,6 +94,7 @@ protected function prepareConfig(#[SensitiveParameter] array $config) : void public function setMemcached(Memcached $memcached) : static { + $this->setByExternal = true; $this->memcached = $memcached; return $this; } @@ -229,10 +230,12 @@ public function close() : bool if ($this->lockId) { $this->memcached->delete($this->lockId); } - if (!$this->memcached->quit()) { - return false; + if ($this->setByExternal === false) { + if (!$this->memcached->quit()) { + return false; + } + $this->memcached = null; } - $this->memcached = null; return true; } @@ -246,7 +249,7 @@ public function destroy($id) : bool && $this->memcached->getResultCode() !== Memcached::RES_NOTFOUND); } - public function gc($max_lifetime) : int | false + public function gc($max_lifetime) : false | int { return 0; } diff --git a/src/SaveHandlers/RedisHandler.php b/src/SaveHandlers/RedisHandler.php index 3b6d35f..8ac9beb 100644 --- a/src/SaveHandlers/RedisHandler.php +++ b/src/SaveHandlers/RedisHandler.php @@ -80,6 +80,7 @@ protected function prepareConfig(#[SensitiveParameter] array $config) : void public function setRedis(Redis $redis) : static { + $this->setByExternal = true; $this->redis = $redis; return $this; } @@ -109,7 +110,7 @@ public function open($path, $name) : bool } $this->redis = new Redis(); try { - $this->redis->connect( + @$this->redis->connect( $this->config['host'], $this->config['port'], $this->config['timeout'] @@ -192,19 +193,21 @@ public function close() : bool if (!isset($this->redis)) { return true; } - try { - if ($this->redis->ping()) { - if ($this->lockId) { - $this->redis->del($this->lockId); - } - if (!$this->redis->close()) { - return false; + if ($this->setByExternal === false) { + try { + if ($this->redis->ping()) { + if ($this->lockId) { + $this->redis->del($this->lockId); + } + if (!$this->redis->close()) { + return false; + } } + } catch (RedisException $e) { + $this->log('Session (redis): Got RedisException on close: ' . $e->getMessage()); } - } catch (RedisException $e) { - $this->log('Session (redis): Got RedisException on close: ' . $e->getMessage()); + $this->redis = null; } - $this->redis = null; return true; } @@ -223,7 +226,7 @@ public function destroy($id) : bool return true; } - public function gc($max_lifetime) : int | false + public function gc($max_lifetime) : false | int { return 0; } diff --git a/src/Session.php b/src/Session.php index 469691f..9941921 100644 --- a/src/Session.php +++ b/src/Session.php @@ -34,7 +34,7 @@ class Session * @param array $options * @param SaveHandler|null $handler */ - public function __construct(array $options = [], SaveHandler $handler = null) + public function __construct(array $options = [], ?SaveHandler $handler = null) { $this->setOptions($options); if ($handler) { @@ -82,8 +82,6 @@ protected function setOptions(array $custom) : void $default = [ 'name' => 'session_id', 'serialize_handler' => $serializer, - 'sid_bits_per_character' => 6, - 'sid_length' => 48, 'cookie_domain' => '', 'cookie_httponly' => 1, 'cookie_lifetime' => 7200, @@ -98,7 +96,12 @@ protected function setOptions(array $custom) : void // used to auto-regenerate the session id: 'auto_regenerate_maxlifetime' => 0, 'auto_regenerate_destroy' => true, + 'set_cookie_permanent' => false, ]; + if (\PHP_VERSION_ID < 80400) { + $default['sid_bits_per_character'] = 6; + $default['sid_length'] = 48; + } $this->options = $custom ? \array_replace($default, $custom) : $default; @@ -116,7 +119,8 @@ protected function getOptions(array $custom = []) : array : $this->options; unset( $options['auto_regenerate_maxlifetime'], - $options['auto_regenerate_destroy'] + $options['auto_regenerate_destroy'], + $options['set_cookie_permanent'], ); return $options; } @@ -144,6 +148,7 @@ public function start(array $customOptions = []) : bool ); } $time = \time(); + $this->setPermanentCookie($time); $this->autoRegenerate($time); $this->clearTemp($time); $this->clearFlash(); @@ -167,6 +172,33 @@ public function activate() : bool return $this->start(); } + /** + * @param int $time + * + * @see https://www.php.net/manual/en/function.session-set-cookie-params.php#100657 + * @see https://stackoverflow.com/a/34252812/6027968 + */ + protected function setPermanentCookie(int $time) : void + { + $setCookie = (bool) $this->options['set_cookie_permanent']; + if ($setCookie === false) { + return; + } + $params = \session_get_cookie_params(); + \setcookie( + \session_name(), // @phpstan-ignore-line + \session_id(), // @phpstan-ignore-line + [ // @phpstan-ignore-line + 'expires' => $time + $this->options['cookie_lifetime'], + 'path' => $params['path'], + 'domain' => $params['domain'], + 'secure' => $params['secure'], + 'httponly' => $params['httponly'], + 'samesite' => $params['samesite'], + ] + ); + } + /** * Auto regenerate the session id. * @@ -553,7 +585,7 @@ public function removeTemp(string $key) : static * $newId is set, it is accepted but not validated. When session_start is * called, the id is only used if it is valid */ - public function id(string $newId = null) : string | false + public function id(?string $newId = null) : false | string { if ($newId !== null && $this->isActive()) { throw new LogicException( @@ -571,7 +603,7 @@ public function id(string $newId = null) : string | false * @return false|int Returns the number of deleted session data for success, * false for failure */ - public function gc() : int | false + public function gc() : false | int { return @\session_gc(); } diff --git a/tests/Debug/SessionCollectionTest.php b/tests/Debug/SessionCollectionTest.php new file mode 100644 index 0000000..5ec53da --- /dev/null +++ b/tests/Debug/SessionCollectionTest.php @@ -0,0 +1,28 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ +namespace Tests\Session\Debug; + +use Framework\Session\Debug\SessionCollection; +use PHPUnit\Framework\TestCase; + +final class SessionCollectionTest extends TestCase +{ + protected SessionCollection $collection; + + protected function setUp() : void + { + $this->collection = new SessionCollection('Session'); + } + + public function testIcon() : void + { + self::assertStringStartsWith('collection->getIcon()); + } +} diff --git a/tests/Debug/SessionCollectorTest.php b/tests/Debug/SessionCollectorTest.php index be5dbbe..486cada 100644 --- a/tests/Debug/SessionCollectorTest.php +++ b/tests/Debug/SessionCollectorTest.php @@ -39,7 +39,7 @@ protected function setUp() : void * * @return Session */ - protected function makeSession(array $options = [], SaveHandler $handler = null) : Session + protected function makeSession(array $options = [], ?SaveHandler $handler = null) : Session { $session = new Session($options, $handler); $session->setDebugCollector($this->collector); @@ -208,7 +208,7 @@ public function destroy($id) : bool return true; } - public function gc($maxLifetime) : int | false + public function gc($maxLifetime) : false | int { return 0; } diff --git a/tests/SaveHandlers/AbstractHandler.php b/tests/SaveHandlers/AbstractHandler.php index 98680ad..58ee232 100644 --- a/tests/SaveHandlers/AbstractHandler.php +++ b/tests/SaveHandlers/AbstractHandler.php @@ -44,12 +44,31 @@ protected function replaceConfig(array $config) : void $this->config = \array_replace_recursive($this->config, $config); } + /** + * @see https://wiki.php.net/rfc/deprecations_php_8_4#sessionsid_length_and_sessionsid_bits_per_character + */ public function testValidateId() : void { + // Ids with PHP lower than 8.4: $id6 = '62my7tSXcbIrOZ-WHsEXhpwUoG,afmBQNGaSBkFN'; $id5 = 'iimuf8lvdectatt5jtkve15831funl8rg5cg6okp'; $id4 = '96aa2c863140e0e714a603cf44b0afc9a0632592'; $this->session->stop(); + /* + * Since PHP 8.4 session.sid_bits_per_character and session.sid_length + * are deprecated. + * + * - The default value for session.sid_bits_per_character is 4 (0-9, a-f). + * - The default value for session.sid_length is 32 character. + */ + if (\PHP_VERSION_ID >= 80400) { + self::assertFalse($this->handler->validateId($id6)); + self::assertFalse($this->handler->validateId($id5)); + self::assertFalse($this->handler->validateId($id4)); + $idPhp84 = '96aa2c863140e0e714a603cf44b0afc9'; + self::assertTrue($this->handler->validateId($idPhp84)); + return; + } \ini_set('session.sid_bits_per_character', '6'); \ini_set('session.sid_length', '40'); self::assertTrue($this->handler->validateId($id6)); diff --git a/tests/SaveHandlers/DatabaseHandlerTest.php b/tests/SaveHandlers/DatabaseHandlerTest.php index 18781c9..9deaba9 100644 --- a/tests/SaveHandlers/DatabaseHandlerTest.php +++ b/tests/SaveHandlers/DatabaseHandlerTest.php @@ -148,7 +148,7 @@ public function unlock() : bool public function testFailToUnlock() : void { $handler = new class($this->config) extends DatabaseHandler { - public string | false $lockId; + public false | string $lockId; public function unlock() : bool { @@ -188,5 +188,7 @@ public function testDatabaseSetterAndGetter() : void $handler->setDatabase($database); self::assertTrue($handler->open('', '')); self::assertSame($database, $handler->getDatabase()); + self::assertTrue($handler->close()); + self::assertSame($database, $handler->getDatabase()); } } diff --git a/tests/SaveHandlers/MemcachedHandlerTest.php b/tests/SaveHandlers/MemcachedHandlerTest.php index a45b875..7669821 100644 --- a/tests/SaveHandlers/MemcachedHandlerTest.php +++ b/tests/SaveHandlers/MemcachedHandlerTest.php @@ -144,7 +144,7 @@ public function testFailToWrite() : void public function testUnlocked() : void { $handler = new class($this->config) extends MemcachedHandler { - public string | false $lockId; + public false | string $lockId; public function unlock() : bool { @@ -160,7 +160,7 @@ public function testReplaceLock() : void { $handler = new class($this->config, $this->logger) extends MemcachedHandler { public ?Memcached $memcached; - public string | false $lockId; + public false | string $lockId; public function lock(string $id) : bool { @@ -176,7 +176,7 @@ public function lock(string $id) : bool public function testFailToDestroy() : void { $handler = new class($this->config) extends MemcachedHandler { - public string | false $lockId; + public false | string $lockId; }; $handler->lockId = false; self::assertFalse($handler->destroy('foo')); @@ -190,5 +190,7 @@ public function testMemcachedSetterAndGetter() : void $handler->setMemcached($memcached); self::assertTrue($handler->open('', '')); self::assertSame($memcached, $handler->getMemcached()); + self::assertTrue($handler->close()); + self::assertSame($memcached, $handler->getMemcached()); } } diff --git a/tests/SaveHandlers/RedisHandlerTest.php b/tests/SaveHandlers/RedisHandlerTest.php index 54a301f..b8eec6e 100644 --- a/tests/SaveHandlers/RedisHandlerTest.php +++ b/tests/SaveHandlers/RedisHandlerTest.php @@ -119,7 +119,7 @@ public function testFailToClose() : void public function testUnlock() : void { $handler = new class($this->config, $this->logger) extends RedisHandler { - public string | false $lockId; + public false | string $lockId; public function unlock() : bool { @@ -145,5 +145,7 @@ public function testRedisSetterAndGetter() : void $handler->setRedis($redis); self::assertTrue($handler->open('', '')); self::assertSame($redis, $handler->getRedis()); + self::assertTrue($handler->close()); + self::assertSame($redis, $handler->getRedis()); } } diff --git a/tests/TestCase.php b/tests/TestCase.php index 3b6100b..c9cd463 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -255,4 +255,50 @@ public function testDestroyCookie() : void { self::assertTrue($this->session->destroyCookie()); } + + /** + * By default, a Set-Cookie header is set when the session starts. + * + * @see TestCase::testSetCookiePermanent() + */ + public function testSetOneCookie() : void + { + $this->session->stop(); + $headers = xdebug_get_headers(); + $count = 0; + foreach ($headers as $header) { + if (\str_starts_with($header, 'Set-Cookie: SessionName=')) { + $count++; + } + } + self::assertSame(1, $count); + } + + /** + * The first time the session is started with the set_cookie_permanent + * option, two identical Set-Cookie headers are set. + * + * In future requests the Set-Cookie header is set only once if the request + * contains the Cookie header with the session name and a valid value (but + * this could not be tested). + * + * @see TestCase::testSetOneCookie() + */ + public function testSetCookiePermanent() : void + { + $this->session->stop(); + $this->session = new Session([ + 'name' => 'sess_id', + 'set_cookie_permanent' => 1, + ], $this->handler); + $this->session->start(); + $headers = xdebug_get_headers(); + $count = 0; + foreach ($headers as $header) { + if (\str_starts_with($header, 'Set-Cookie: sess_id=')) { + $count++; + } + } + self::assertSame(2, $count); + } }