From f493f7f158fa0089095becf2b042982bfb61d730 Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 21 Dec 2022 15:18:22 +0900 Subject: [PATCH 1/3] fix: remove Session config items in Config\App --- .../Generators/MigrationGenerator.php | 6 +--- system/Config/Services.php | 17 +++++---- system/Session/Handlers/BaseHandler.php | 29 ++++++++------- system/Session/Handlers/DatabaseHandler.php | 21 +++-------- system/Session/Handlers/FileHandler.php | 6 ++-- system/Session/Handlers/MemcachedHandler.php | 14 +++----- system/Session/Handlers/RedisHandler.php | 27 ++++---------- system/Session/Session.php | 35 +++++++------------ system/Test/CIUnitTestCase.php | 10 ++++-- tests/system/CommonFunctionsTest.php | 29 ++++----------- .../SecurityCSRFSessionRandomizeTokenTest.php | 35 +++++++++---------- .../Security/SecurityCSRFSessionTest.php | 35 +++++++++---------- .../Handlers/Database/MySQLiHandlerTest.php | 8 ++--- .../Handlers/Database/PostgreHandlerTest.php | 8 ++--- .../Handlers/Database/RedisHandlerTest.php | 8 ++--- tests/system/Session/SessionTest.php | 16 ++++----- 16 files changed, 116 insertions(+), 188 deletions(-) diff --git a/system/Commands/Generators/MigrationGenerator.php b/system/Commands/Generators/MigrationGenerator.php index 5cc3b6dfd56a..a47e34196063 100644 --- a/system/Commands/Generators/MigrationGenerator.php +++ b/system/Commands/Generators/MigrationGenerator.php @@ -14,7 +14,6 @@ use CodeIgniter\CLI\BaseCommand; use CodeIgniter\CLI\CLI; use CodeIgniter\CLI\GeneratorTrait; -use Config\App as AppConfig; use Config\Session as SessionConfig; /** @@ -108,13 +107,10 @@ protected function prepare(string $class): string $data['DBGroup'] = is_string($DBGroup) ? $DBGroup : 'default'; $data['DBDriver'] = config('Database')->{$data['DBGroup']}['DBDriver']; - /** @var AppConfig $config */ - $config = config('App'); /** @var SessionConfig|null $session */ $session = config('Session'); - $data['matchIP'] = ($session instanceof SessionConfig) - ? $session->matchIP : $config->sessionMatchIP; + $data['matchIP'] = $session->matchIP; } return $this->parseTemplate($class, [], [], $data); diff --git a/system/Config/Services.php b/system/Config/Services.php index a2d13c40e80c..5edd93ca693e 100644 --- a/system/Config/Services.php +++ b/system/Config/Services.php @@ -634,6 +634,8 @@ public static function security(?App $config = null, bool $getShared = true) * Return the session manager. * * @return Session + * + * @TODO replace the first parameter type `?App` with `?SessionConfig` */ public static function session(?App $config = null, bool $getShared = true) { @@ -641,18 +643,14 @@ public static function session(?App $config = null, bool $getShared = true) return static::getSharedInstance('session', $config); } - $config ??= config('App'); - assert($config instanceof App); - - $logger = AppServices::logger(); - - /** @var SessionConfig|null $sessionConfig */ - $sessionConfig = config('Session'); + /** @var SessionConfig $config */ + $config = config('Session'); + assert($config instanceof SessionConfig, 'Missing "Config/Session.php".'); - $driverName = $sessionConfig->driver ?? $config->sessionDriver; + $driverName = $config->driver; if ($driverName === DatabaseHandler::class) { - $DBGroup = $sessionConfig->DBGroup ?? $config->sessionDBGroup ?? config(Database::class)->defaultGroup; + $DBGroup = $config->DBGroup ?? config(Database::class)->defaultGroup; $db = Database::connect($DBGroup); $driver = $db->getPlatform(); @@ -664,6 +662,7 @@ public static function session(?App $config = null, bool $getShared = true) } } + $logger = AppServices::logger(); $driver = new $driverName($config, AppServices::request()->getIPAddress()); $driver->setLogger($logger); diff --git a/system/Session/Handlers/BaseHandler.php b/system/Session/Handlers/BaseHandler.php index f7d6ff1a90c7..8b250cfa05dd 100644 --- a/system/Session/Handlers/BaseHandler.php +++ b/system/Session/Handlers/BaseHandler.php @@ -105,26 +105,19 @@ abstract class BaseHandler implements SessionHandlerInterface */ protected $ipAddress; - public function __construct(AppConfig $config, string $ipAddress) + public function __construct(SessionConfig $session, string $ipAddress) { - /** @var SessionConfig|null $session */ - $session = config('Session'); - // Store Session configurations - if ($session instanceof SessionConfig) { - $this->cookieName = $session->cookieName; - $this->matchIP = $session->matchIP; - $this->savePath = $session->savePath; - } else { - // `Config/Session.php` is absence - $this->cookieName = $config->sessionCookieName; - $this->matchIP = $config->sessionMatchIP; - $this->savePath = $config->sessionSavePath; - } + $this->cookieName = $session->cookieName; + $this->matchIP = $session->matchIP; + $this->savePath = $session->savePath; /** @var CookieConfig|null $cookie */ $cookie = config('Cookie'); + /** @var AppConfig $config */ + $config = config('App'); + if ($cookie instanceof CookieConfig) { // Session cookies have no prefix. $this->cookieDomain = $cookie->domain; @@ -151,7 +144,13 @@ protected function destroyCookie(): bool return setcookie( $this->cookieName, '', - ['expires' => 1, 'path' => $this->cookiePath, 'domain' => $this->cookieDomain, 'secure' => $this->cookieSecure, 'httponly' => true] + [ + 'expires' => 1, + 'path' => $this->cookiePath, + 'domain' => $this->cookieDomain, + 'secure' => $this->cookieSecure, + 'httponly' => true, + ] ); } diff --git a/system/Session/Handlers/DatabaseHandler.php b/system/Session/Handlers/DatabaseHandler.php index 42be98ee96e9..7be2c8532a7b 100644 --- a/system/Session/Handlers/DatabaseHandler.php +++ b/system/Session/Handlers/DatabaseHandler.php @@ -14,7 +14,6 @@ use CodeIgniter\Database\BaseBuilder; use CodeIgniter\Database\BaseConnection; use CodeIgniter\Session\Exceptions\SessionException; -use Config\App as AppConfig; use Config\Database; use Config\Session as SessionConfig; use ReturnTypeWillChange; @@ -69,24 +68,14 @@ class DatabaseHandler extends BaseHandler /** * @throws SessionException */ - public function __construct(AppConfig $config, string $ipAddress) + public function __construct(SessionConfig $session, string $ipAddress) { - parent::__construct($config, $ipAddress); - - /** @var SessionConfig|null $session */ - $session = config('Session'); + parent::__construct($session, $ipAddress); // Store Session configurations - if ($session instanceof SessionConfig) { - $this->DBGroup = $session->DBGroup ?? config(Database::class)->defaultGroup; - // Add sessionCookieName for multiple session cookies. - $this->idPrefix = $session->cookieName . ':'; - } else { - // `Config/Session.php` is absence - $this->DBGroup = $config->sessionDBGroup ?? config(Database::class)->defaultGroup; - // Add sessionCookieName for multiple session cookies. - $this->idPrefix = $config->sessionCookieName . ':'; - } + // Add sessionCookieName for multiple session cookies. + $this->idPrefix = $session->cookieName . ':'; + $this->DBGroup = $session->DBGroup ?? config(Database::class)->defaultGroup; $this->table = $this->savePath; if (empty($this->table)) { diff --git a/system/Session/Handlers/FileHandler.php b/system/Session/Handlers/FileHandler.php index cc8a6694f692..013dd4b6c0ec 100644 --- a/system/Session/Handlers/FileHandler.php +++ b/system/Session/Handlers/FileHandler.php @@ -13,7 +13,7 @@ use CodeIgniter\I18n\Time; use CodeIgniter\Session\Exceptions\SessionException; -use Config\App as AppConfig; +use Config\Session as SessionConfig; use ReturnTypeWillChange; /** @@ -63,9 +63,9 @@ class FileHandler extends BaseHandler */ protected $sessionIDRegex = ''; - public function __construct(AppConfig $config, string $ipAddress) + public function __construct(SessionConfig $session, string $ipAddress) { - parent::__construct($config, $ipAddress); + parent::__construct($session, $ipAddress); if (! empty($this->savePath)) { $this->savePath = rtrim($this->savePath, '/\\'); diff --git a/system/Session/Handlers/MemcachedHandler.php b/system/Session/Handlers/MemcachedHandler.php index e4c8c6e8c673..a6597613b2fd 100644 --- a/system/Session/Handlers/MemcachedHandler.php +++ b/system/Session/Handlers/MemcachedHandler.php @@ -13,7 +13,6 @@ use CodeIgniter\I18n\Time; use CodeIgniter\Session\Exceptions\SessionException; -use Config\App as AppConfig; use Config\Session as SessionConfig; use Memcached; use ReturnTypeWillChange; @@ -54,23 +53,18 @@ class MemcachedHandler extends BaseHandler /** * @throws SessionException */ - public function __construct(AppConfig $config, string $ipAddress) + public function __construct(SessionConfig $session, string $ipAddress) { - parent::__construct($config, $ipAddress); + parent::__construct($session, $ipAddress); - /** @var SessionConfig|null $session */ - $session = config('Session'); - - $this->sessionExpiration = ($session instanceof SessionConfig) - ? $session->expiration : $config->sessionExpiration; + $this->sessionExpiration = $session->expiration; if (empty($this->savePath)) { throw SessionException::forEmptySavepath(); } // Add sessionCookieName for multiple session cookies. - $this->keyPrefix .= ($session instanceof SessionConfig) - ? $session->cookieName : $config->sessionCookieName . ':'; + $this->keyPrefix .= $session->cookieName . ':'; if ($this->matchIP === true) { $this->keyPrefix .= $this->ipAddress . ':'; diff --git a/system/Session/Handlers/RedisHandler.php b/system/Session/Handlers/RedisHandler.php index 4492c4dbed32..806156286ba4 100644 --- a/system/Session/Handlers/RedisHandler.php +++ b/system/Session/Handlers/RedisHandler.php @@ -13,7 +13,6 @@ use CodeIgniter\I18n\Time; use CodeIgniter\Session\Exceptions\SessionException; -use Config\App as AppConfig; use Config\Session as SessionConfig; use Redis; use RedisException; @@ -66,28 +65,16 @@ class RedisHandler extends BaseHandler * * @throws SessionException */ - public function __construct(AppConfig $config, string $ipAddress) + public function __construct(SessionConfig $session, string $ipAddress) { - parent::__construct($config, $ipAddress); - - /** @var SessionConfig|null $session */ - $session = config('Session'); + parent::__construct($session, $ipAddress); // Store Session configurations - if ($session instanceof SessionConfig) { - $this->sessionExpiration = empty($session->expiration) - ? (int) ini_get('session.gc_maxlifetime') - : (int) $session->expiration; - // Add sessionCookieName for multiple session cookies. - $this->keyPrefix .= $session->cookieName . ':'; - } else { - // `Config/Session.php` is absence - $this->sessionExpiration = empty($config->sessionExpiration) - ? (int) ini_get('session.gc_maxlifetime') - : (int) $config->sessionExpiration; - // Add sessionCookieName for multiple session cookies. - $this->keyPrefix .= $config->sessionCookieName . ':'; - } + // Add sessionCookieName for multiple session cookies. + $this->keyPrefix .= $session->cookieName . ':'; + $this->sessionExpiration = empty($session->expiration) + ? (int) ini_get('session.gc_maxlifetime') + : (int) $session->expiration; $this->setSavePath(); diff --git a/system/Session/Session.php b/system/Session/Session.php index 497111b60030..70afefce27ca 100644 --- a/system/Session/Session.php +++ b/system/Session/Session.php @@ -161,32 +161,21 @@ class Session implements SessionInterface * * Extract configuration settings and save them here. */ - public function __construct(SessionHandlerInterface $driver, App $config) + public function __construct(SessionHandlerInterface $driver, SessionConfig $session) { $this->driver = $driver; - /** @var SessionConfig|null $session */ - $session = config('Session'); - // Store Session configurations - if ($session instanceof SessionConfig) { - $this->sessionDriverName = $session->driver; - $this->sessionCookieName = $session->cookieName ?? $this->sessionCookieName; - $this->sessionExpiration = $session->expiration ?? $this->sessionExpiration; - $this->sessionSavePath = $session->savePath; - $this->sessionMatchIP = $session->matchIP ?? $this->sessionMatchIP; - $this->sessionTimeToUpdate = $session->timeToUpdate ?? $this->sessionTimeToUpdate; - $this->sessionRegenerateDestroy = $session->regenerateDestroy ?? $this->sessionRegenerateDestroy; - } else { - // `Config/Session.php` is absence - $this->sessionDriverName = $config->sessionDriver; - $this->sessionCookieName = $config->sessionCookieName ?? $this->sessionCookieName; - $this->sessionExpiration = $config->sessionExpiration ?? $this->sessionExpiration; - $this->sessionSavePath = $config->sessionSavePath; - $this->sessionMatchIP = $config->sessionMatchIP ?? $this->sessionMatchIP; - $this->sessionTimeToUpdate = $config->sessionTimeToUpdate ?? $this->sessionTimeToUpdate; - $this->sessionRegenerateDestroy = $config->sessionRegenerateDestroy ?? $this->sessionRegenerateDestroy; - } + $this->sessionDriverName = $session->driver; + $this->sessionCookieName = $session->cookieName ?? $this->sessionCookieName; + $this->sessionExpiration = $session->expiration ?? $this->sessionExpiration; + $this->sessionSavePath = $session->savePath; + $this->sessionMatchIP = $session->matchIP ?? $this->sessionMatchIP; + $this->sessionTimeToUpdate = $session->timeToUpdate ?? $this->sessionTimeToUpdate; + $this->sessionRegenerateDestroy = $session->regenerateDestroy ?? $this->sessionRegenerateDestroy; + + /** @var App $config */ + $config = config('App'); // DEPRECATED COOKIE MANAGEMENT $this->cookiePath = $config->cookiePath ?? $this->cookiePath; @@ -194,7 +183,7 @@ public function __construct(SessionHandlerInterface $driver, App $config) $this->cookieSecure = $config->cookieSecure ?? $this->cookieSecure; $this->cookieSameSite = $config->cookieSameSite ?? $this->cookieSameSite; - /** @var CookieConfig $cookie */ + /** @var CookieConfig|null $cookie */ $cookie = config('Cookie'); $this->cookie = (new Cookie($this->sessionCookieName, '', [ diff --git a/system/Test/CIUnitTestCase.php b/system/Test/CIUnitTestCase.php index bfde56faf6ff..4f2d1715049d 100644 --- a/system/Test/CIUnitTestCase.php +++ b/system/Test/CIUnitTestCase.php @@ -27,6 +27,7 @@ use Config\Autoload; use Config\Modules; use Config\Services; +use Config\Session; use Exception; use PHPUnit\Framework\TestCase; @@ -333,8 +334,13 @@ protected function mockSession() { $_SESSION = []; - $config = config('App'); - $session = new MockSession(new ArrayHandler($config, '0.0.0.0'), $config); + /** @var Session $sessionConfig */ + $sessionConfig = config('Session'); + + $session = new MockSession( + new ArrayHandler($sessionConfig, '0.0.0.0'), + $sessionConfig + ); Services::injectMock('session', $session); } diff --git a/tests/system/CommonFunctionsTest.php b/tests/system/CommonFunctionsTest.php index 2e8c3be0d625..4d710da1d0b6 100644 --- a/tests/system/CommonFunctionsTest.php +++ b/tests/system/CommonFunctionsTest.php @@ -31,6 +31,7 @@ use Config\Logger; use Config\Modules; use Config\Services; +use Config\Session as SessionConfig; use Kint; use RuntimeException; use stdClass; @@ -483,7 +484,6 @@ public function testSlashItem() $this->assertSame('/', slash_item('cookiePath')); // / $this->assertSame('', slash_item('cookieDomain')); // '' $this->assertSame('en/', slash_item('defaultLocale')); // en - $this->assertSame('7200/', slash_item('sessionExpiration')); // int 7200 $this->assertSame('', slash_item('negotiateLocale')); // false $this->assertSame('1/', slash_item('cookieHTTPOnly')); // true } @@ -506,28 +506,11 @@ public function testSlashItemThrowsErrorOnNonStringableItem() protected function injectSessionMock() { - $defaults = [ - 'sessionDriver' => FileHandler::class, - 'sessionCookieName' => 'ci_session', - 'sessionExpiration' => 7200, - 'sessionSavePath' => '', - 'sessionMatchIP' => false, - 'sessionTimeToUpdate' => 300, - 'sessionRegenerateDestroy' => false, - 'cookieDomain' => '', - 'cookiePrefix' => '', - 'cookiePath' => '/', - 'cookieSecure' => false, - 'cookieSameSite' => 'Lax', - ]; - - $appConfig = new App(); - - foreach ($defaults as $key => $config) { - $appConfig->{$key} = $config; - } - - $session = new MockSession(new FileHandler($appConfig, '127.0.0.1'), $appConfig); + $sessionConfig = new SessionConfig(); + $session = new MockSession( + new FileHandler($sessionConfig, '127.0.0.1'), + $sessionConfig + ); $session->setLogger(new TestLogger(new Logger())); BaseService::injectMock('session', $session); } diff --git a/tests/system/Security/SecurityCSRFSessionRandomizeTokenTest.php b/tests/system/Security/SecurityCSRFSessionRandomizeTokenTest.php index 6086472d3f4c..8ebadf3ba7c5 100644 --- a/tests/system/Security/SecurityCSRFSessionRandomizeTokenTest.php +++ b/tests/system/Security/SecurityCSRFSessionRandomizeTokenTest.php @@ -17,7 +17,6 @@ use CodeIgniter\HTTP\URI; use CodeIgniter\HTTP\UserAgent; use CodeIgniter\Security\Exceptions\SecurityException; -use CodeIgniter\Session\Handlers\ArrayHandler; use CodeIgniter\Session\Handlers\FileHandler; use CodeIgniter\Session\Session; use CodeIgniter\Test\CIUnitTestCase; @@ -25,9 +24,9 @@ use CodeIgniter\Test\Mock\MockSecurity; use CodeIgniter\Test\Mock\MockSession; use CodeIgniter\Test\TestLogger; -use Config\App as AppConfig; use Config\Logger as LoggerConfig; use Config\Security as SecurityConfig; +use Config\Session as SessionConfig; /** * @runTestsInSeparateProcesses @@ -68,28 +67,26 @@ protected function setUp(): void private function createSession($options = []): Session { $defaults = [ - 'sessionDriver' => FileHandler::class, - 'sessionCookieName' => 'ci_session', - 'sessionExpiration' => 7200, - 'sessionSavePath' => '', - 'sessionMatchIP' => false, - 'sessionTimeToUpdate' => 300, - 'sessionRegenerateDestroy' => false, - 'cookieDomain' => '', - 'cookiePrefix' => '', - 'cookiePath' => '/', - 'cookieSecure' => false, - 'cookieSameSite' => 'Lax', + 'driver' => FileHandler::class, + 'cookieName' => 'ci_session', + 'expiration' => 7200, + 'savePath' => '', + 'matchIP' => false, + 'timeToUpdate' => 300, + 'regenerateDestroy' => false, ]; + $config = array_merge($defaults, $options); - $config = array_merge($defaults, $options); - $appConfig = new AppConfig(); + $sessionConfig = new SessionConfig(); - foreach ($config as $key => $c) { - $appConfig->{$key} = $c; + foreach ($config as $key => $value) { + $sessionConfig->{$key} = $value; } - $session = new MockSession(new ArrayHandler($appConfig, '127.0.0.1'), $appConfig); + $session = new MockSession( + new FileHandler($sessionConfig, '127.0.0.1'), + $sessionConfig + ); $session->setLogger(new TestLogger(new LoggerConfig())); return $session; diff --git a/tests/system/Security/SecurityCSRFSessionTest.php b/tests/system/Security/SecurityCSRFSessionTest.php index 7fbd7096853f..85ad6049491f 100644 --- a/tests/system/Security/SecurityCSRFSessionTest.php +++ b/tests/system/Security/SecurityCSRFSessionTest.php @@ -17,16 +17,15 @@ use CodeIgniter\HTTP\URI; use CodeIgniter\HTTP\UserAgent; use CodeIgniter\Security\Exceptions\SecurityException; -use CodeIgniter\Session\Handlers\ArrayHandler; use CodeIgniter\Session\Handlers\FileHandler; use CodeIgniter\Session\Session; use CodeIgniter\Test\CIUnitTestCase; use CodeIgniter\Test\Mock\MockAppConfig; use CodeIgniter\Test\Mock\MockSession; use CodeIgniter\Test\TestLogger; -use Config\App as AppConfig; use Config\Logger as LoggerConfig; use Config\Security as SecurityConfig; +use Config\Session as SessionConfig; /** * @runTestsInSeparateProcesses @@ -61,28 +60,26 @@ protected function setUp(): void private function createSession($options = []): Session { $defaults = [ - 'sessionDriver' => FileHandler::class, - 'sessionCookieName' => 'ci_session', - 'sessionExpiration' => 7200, - 'sessionSavePath' => '', - 'sessionMatchIP' => false, - 'sessionTimeToUpdate' => 300, - 'sessionRegenerateDestroy' => false, - 'cookieDomain' => '', - 'cookiePrefix' => '', - 'cookiePath' => '/', - 'cookieSecure' => false, - 'cookieSameSite' => 'Lax', + 'driver' => FileHandler::class, + 'cookieName' => 'ci_session', + 'expiration' => 7200, + 'savePath' => '', + 'matchIP' => false, + 'timeToUpdate' => 300, + 'regenerateDestroy' => false, ]; + $config = array_merge($defaults, $options); - $config = array_merge($defaults, $options); - $appConfig = new AppConfig(); + $sessionConfig = new SessionConfig(); - foreach ($config as $key => $c) { - $appConfig->{$key} = $c; + foreach ($config as $key => $value) { + $sessionConfig->{$key} = $value; } - $session = new MockSession(new ArrayHandler($appConfig, '127.0.0.1'), $appConfig); + $session = new MockSession( + new FileHandler($sessionConfig, '127.0.0.1'), + $sessionConfig + ); $session->setLogger(new TestLogger(new LoggerConfig())); return $session; diff --git a/tests/system/Session/Handlers/Database/MySQLiHandlerTest.php b/tests/system/Session/Handlers/Database/MySQLiHandlerTest.php index b18f1b047eef..2d0068178bdd 100644 --- a/tests/system/Session/Handlers/Database/MySQLiHandlerTest.php +++ b/tests/system/Session/Handlers/Database/MySQLiHandlerTest.php @@ -11,8 +11,6 @@ namespace CodeIgniter\Session\Handlers\Database; -use CodeIgniter\Config\Factories; -use Config\App as AppConfig; use Config\Database as DatabaseConfig; use Config\Session as SessionConfig; @@ -43,14 +41,14 @@ protected function getInstance($options = []) 'timeToUpdate' => 300, 'regenerateDestroy' => false, ]; + $config = array_merge($defaults, $options); + $sessionConfig = new SessionConfig(); - $config = array_merge($defaults, $options); foreach ($config as $key => $value) { $sessionConfig->{$key} = $value; } - Factories::injectMock('config', 'Session', $sessionConfig); - return new MySQLiHandler(new AppConfig(), $this->userIpAddress); + return new MySQLiHandler($sessionConfig, $this->userIpAddress); } } diff --git a/tests/system/Session/Handlers/Database/PostgreHandlerTest.php b/tests/system/Session/Handlers/Database/PostgreHandlerTest.php index f9db847faab0..bd89ce87aa7a 100644 --- a/tests/system/Session/Handlers/Database/PostgreHandlerTest.php +++ b/tests/system/Session/Handlers/Database/PostgreHandlerTest.php @@ -11,8 +11,6 @@ namespace CodeIgniter\Session\Handlers\Database; -use CodeIgniter\Config\Factories; -use Config\App as AppConfig; use Config\Database as DatabaseConfig; use Config\Session as SessionConfig; @@ -43,14 +41,14 @@ protected function getInstance($options = []) 'timeToUpdate' => 300, 'regenerateDestroy' => false, ]; + $config = array_merge($defaults, $options); + $sessionConfig = new SessionConfig(); - $config = array_merge($defaults, $options); foreach ($config as $key => $value) { $sessionConfig->{$key} = $value; } - Factories::injectMock('config', 'Session', $sessionConfig); - return new PostgreHandler(new AppConfig(), $this->userIpAddress); + return new PostgreHandler($sessionConfig, $this->userIpAddress); } } diff --git a/tests/system/Session/Handlers/Database/RedisHandlerTest.php b/tests/system/Session/Handlers/Database/RedisHandlerTest.php index 4a4cdcc45f19..12e18ec05dca 100644 --- a/tests/system/Session/Handlers/Database/RedisHandlerTest.php +++ b/tests/system/Session/Handlers/Database/RedisHandlerTest.php @@ -11,10 +11,8 @@ namespace CodeIgniter\Session\Handlers\Database; -use CodeIgniter\Config\Factories; use CodeIgniter\Session\Handlers\RedisHandler; use CodeIgniter\Test\CIUnitTestCase; -use Config\App as AppConfig; use Config\Session as SessionConfig; use Redis; @@ -43,15 +41,15 @@ protected function getInstance($options = []) 'timeToUpdate' => 300, 'regenerateDestroy' => false, ]; + $config = array_merge($defaults, $options); + $sessionConfig = new SessionConfig(); - $config = array_merge($defaults, $options); foreach ($config as $key => $value) { $sessionConfig->{$key} = $value; } - Factories::injectMock('config', 'Session', $sessionConfig); - return new RedisHandler(new AppConfig(), $this->userIpAddress); + return new RedisHandler($sessionConfig, $this->userIpAddress); } public function testSavePathTimeoutFloat() diff --git a/tests/system/Session/SessionTest.php b/tests/system/Session/SessionTest.php index 8e0170d0fee1..bb53bb751e0b 100644 --- a/tests/system/Session/SessionTest.php +++ b/tests/system/Session/SessionTest.php @@ -17,7 +17,6 @@ use CodeIgniter\Test\CIUnitTestCase; use CodeIgniter\Test\Mock\MockSession; use CodeIgniter\Test\TestLogger; -use Config\App as AppConfig; use Config\Cookie as CookieConfig; use Config\Logger as LoggerConfig; use Config\Session as SessionConfig; @@ -43,8 +42,6 @@ protected function setUp(): void protected function getInstance($options = []) { - $appConfig = new AppConfig(); - $defaults = [ 'driver' => FileHandler::class, 'cookieName' => 'ci_session', @@ -54,15 +51,18 @@ protected function getInstance($options = []) 'timeToUpdate' => 300, 'regenerateDestroy' => false, ]; + $config = array_merge($defaults, $options); + $sessionConfig = new SessionConfig(); - $config = array_merge($defaults, $options); foreach ($config as $key => $value) { $sessionConfig->{$key} = $value; } - Factories::injectMock('config', 'Session', $sessionConfig); - $session = new MockSession(new FileHandler($appConfig, '127.0.0.1'), $appConfig); + $session = new MockSession( + new FileHandler($sessionConfig, '127.0.0.1'), + $sessionConfig + ); $session->setLogger(new TestLogger(new LoggerConfig())); return $session; @@ -572,6 +572,7 @@ public function testLaxSameSite() $session = $this->getInstance(); $session->start(); + $cookies = $session->cookies; $this->assertCount(1, $cookies); $this->assertSame('Lax', $cookies[0]->getSameSite()); @@ -582,7 +583,6 @@ public function testNoneSameSite() $config = new CookieConfig(); $config->secure = true; $config->samesite = 'None'; - Factories::injectMock('config', CookieConfig::class, $config); $session = $this->getInstance(); @@ -597,7 +597,6 @@ public function testNoSameSiteReturnsDefault() { $config = new CookieConfig(); $config->samesite = ''; - Factories::injectMock('config', CookieConfig::class, $config); $session = $this->getInstance(); @@ -615,7 +614,6 @@ public function testInvalidSameSite() $config = new CookieConfig(); $config->samesite = 'Invalid'; - Factories::injectMock('config', CookieConfig::class, $config); $session = $this->getInstance(); From 07f92b921af6e145df68b826b2236c9887097333 Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 21 Dec 2022 15:45:12 +0900 Subject: [PATCH 2/3] refactor: add property for SessionConfig and use it --- phpstan-baseline.neon.dist | 5 --- system/Session/Session.php | 77 +++++++++++++++++++------------- system/Test/Mock/MockSession.php | 2 +- 3 files changed, 47 insertions(+), 37 deletions(-) diff --git a/phpstan-baseline.neon.dist b/phpstan-baseline.neon.dist index 986601771fef..4f83a4ea0c80 100644 --- a/phpstan-baseline.neon.dist +++ b/phpstan-baseline.neon.dist @@ -295,11 +295,6 @@ parameters: count: 1 path: system/Session/Handlers/RedisHandler.php - - - message: "#^Property CodeIgniter\\\\Session\\\\Session\\:\\:\\$sessionExpiration \\(int\\) in isset\\(\\) is not nullable\\.$#" - count: 1 - path: system/Session/Session.php - - message: "#^Negated boolean expression is always false\\.$#" count: 1 diff --git a/system/Session/Session.php b/system/Session/Session.php index 70afefce27ca..7d353c92a196 100644 --- a/system/Session/Session.php +++ b/system/Session/Session.php @@ -43,6 +43,8 @@ class Session implements SessionInterface * The storage driver to use: files, database, redis, memcached * * @var string + * + * @deprecated Use $this->config->driver. */ protected $sessionDriverName; @@ -50,6 +52,8 @@ class Session implements SessionInterface * The session cookie name, must contain only [0-9a-z_-] characters. * * @var string + * + * @deprecated Use $this->config->cookieName. */ protected $sessionCookieName = 'ci_session'; @@ -58,11 +62,13 @@ class Session implements SessionInterface * Setting it to 0 (zero) means expire when the browser is closed. * * @var int + * + * @deprecated Use $this->config->expiration. */ protected $sessionExpiration = 7200; /** - * The location to save sessions to, driver dependent.. + * The location to save sessions to, driver dependent. * * For the 'files' driver, it's a path to a writable directory. * WARNING: Only absolute paths are supported! @@ -74,6 +80,8 @@ class Session implements SessionInterface * IMPORTANT: You are REQUIRED to set a valid save path! * * @var string + * + * @deprecated Use $this->config->savePath. */ protected $sessionSavePath; @@ -84,6 +92,8 @@ class Session implements SessionInterface * your session table's PRIMARY KEY when changing this setting. * * @var bool + * + * @deprecated Use $this->config->matchIP. */ protected $sessionMatchIP = false; @@ -91,6 +101,8 @@ class Session implements SessionInterface * How many seconds between CI regenerating the session ID. * * @var int + * + * @deprecated Use $this->config->timeToUpdate. */ protected $sessionTimeToUpdate = 300; @@ -100,6 +112,8 @@ class Session implements SessionInterface * will be later deleted by the garbage collector. * * @var bool + * + * @deprecated Use $this->config->regenerateDestroy. */ protected $sessionRegenerateDestroy = false; @@ -156,6 +170,11 @@ class Session implements SessionInterface */ protected $sidRegexp; + /** + * Session Config + */ + protected SessionConfig $config; + /** * Constructor. * @@ -165,14 +184,16 @@ public function __construct(SessionHandlerInterface $driver, SessionConfig $sess { $this->driver = $driver; + $this->config = $session; + // Store Session configurations $this->sessionDriverName = $session->driver; - $this->sessionCookieName = $session->cookieName ?? $this->sessionCookieName; - $this->sessionExpiration = $session->expiration ?? $this->sessionExpiration; + $this->sessionCookieName = $session->cookieName; + $this->sessionExpiration = $session->expiration; $this->sessionSavePath = $session->savePath; - $this->sessionMatchIP = $session->matchIP ?? $this->sessionMatchIP; - $this->sessionTimeToUpdate = $session->timeToUpdate ?? $this->sessionTimeToUpdate; - $this->sessionRegenerateDestroy = $session->regenerateDestroy ?? $this->sessionRegenerateDestroy; + $this->sessionMatchIP = $session->matchIP; + $this->sessionTimeToUpdate = $session->timeToUpdate; + $this->sessionRegenerateDestroy = $session->regenerateDestroy; /** @var App $config */ $config = config('App'); @@ -186,8 +207,8 @@ public function __construct(SessionHandlerInterface $driver, SessionConfig $sess /** @var CookieConfig|null $cookie */ $cookie = config('Cookie'); - $this->cookie = (new Cookie($this->sessionCookieName, '', [ - 'expires' => $this->sessionExpiration === 0 ? 0 : Time::now()->getTimestamp() + $this->sessionExpiration, + $this->cookie = (new Cookie($this->config->cookieName, '', [ + 'expires' => $this->config->expiration === 0 ? 0 : Time::now()->getTimestamp() + $this->config->expiration, 'path' => $cookie->path ?? $config->cookiePath, 'domain' => $cookie->domain ?? $config->cookieDomain, 'secure' => $cookie->secure ?? $config->cookieSecure, @@ -230,32 +251,32 @@ public function start() $this->setSaveHandler(); // Sanitize the cookie, because apparently PHP doesn't do that for userspace handlers - if (isset($_COOKIE[$this->sessionCookieName]) - && (! is_string($_COOKIE[$this->sessionCookieName]) || ! preg_match('#\A' . $this->sidRegexp . '\z#', $_COOKIE[$this->sessionCookieName])) + if (isset($_COOKIE[$this->config->cookieName]) + && (! is_string($_COOKIE[$this->config->cookieName]) || ! preg_match('#\A' . $this->sidRegexp . '\z#', $_COOKIE[$this->config->cookieName])) ) { - unset($_COOKIE[$this->sessionCookieName]); + unset($_COOKIE[$this->config->cookieName]); } $this->startSession(); // Is session ID auto-regeneration configured? (ignoring ajax requests) if ((empty($_SERVER['HTTP_X_REQUESTED_WITH']) || strtolower($_SERVER['HTTP_X_REQUESTED_WITH']) !== 'xmlhttprequest') - && ($regenerateTime = $this->sessionTimeToUpdate) > 0 + && ($regenerateTime = $this->config->timeToUpdate) > 0 ) { if (! isset($_SESSION['__ci_last_regenerate'])) { $_SESSION['__ci_last_regenerate'] = Time::now()->getTimestamp(); } elseif ($_SESSION['__ci_last_regenerate'] < (Time::now()->getTimestamp() - $regenerateTime)) { - $this->regenerate((bool) $this->sessionRegenerateDestroy); + $this->regenerate((bool) $this->config->regenerateDestroy); } } // Another work-around ... PHP doesn't seem to send the session cookie // unless it is being currently created or regenerated - elseif (isset($_COOKIE[$this->sessionCookieName]) && $_COOKIE[$this->sessionCookieName] === session_id()) { + elseif (isset($_COOKIE[$this->config->cookieName]) && $_COOKIE[$this->config->cookieName] === session_id()) { $this->setCookie(); } $this->initVars(); - $this->logger->info("Session: Class initialized using '" . $this->sessionDriverName . "' driver."); + $this->logger->info("Session: Class initialized using '" . $this->config->driver . "' driver."); return $this; } @@ -270,7 +291,7 @@ public function start() public function stop() { setcookie( - $this->sessionCookieName, + $this->config->cookieName, session_id(), ['expires' => 1, 'path' => $this->cookie->getPath(), 'domain' => $this->cookie->getDomain(), 'secure' => $this->cookie->isSecure(), 'httponly' => true] ); @@ -285,16 +306,12 @@ public function stop() */ protected function configure() { - if (empty($this->sessionCookieName)) { - $this->sessionCookieName = ini_get('session.name'); - } else { - ini_set('session.name', $this->sessionCookieName); - } + ini_set('session.name', $this->config->cookieName); $sameSite = $this->cookie->getSameSite() ?: ucfirst(Cookie::SAMESITE_LAX); $params = [ - 'lifetime' => $this->sessionExpiration, + 'lifetime' => $this->config->expiration, 'path' => $this->cookie->getPath(), 'domain' => $this->cookie->getDomain(), 'secure' => $this->cookie->isSecure(), @@ -305,14 +322,12 @@ protected function configure() ini_set('session.cookie_samesite', $sameSite); session_set_cookie_params($params); - if (! isset($this->sessionExpiration)) { - $this->sessionExpiration = (int) ini_get('session.gc_maxlifetime'); - } elseif ($this->sessionExpiration > 0) { - ini_set('session.gc_maxlifetime', (string) $this->sessionExpiration); + if ($this->config->expiration > 0) { + ini_set('session.gc_maxlifetime', (string) $this->config->expiration); } - if (! empty($this->sessionSavePath)) { - ini_set('session.save_path', $this->sessionSavePath); + if (! empty($this->config->savePath)) { + ini_set('session.save_path', $this->config->savePath); } // Security is king @@ -419,12 +434,12 @@ private function removeOldSessionCookie(): void $response = Services::response(); $cookieStoreInResponse = $response->getCookieStore(); - if (! $cookieStoreInResponse->has($this->sessionCookieName)) { + if (! $cookieStoreInResponse->has($this->config->cookieName)) { return; } // CookieStore is immutable. - $newCookieStore = $cookieStoreInResponse->remove($this->sessionCookieName); + $newCookieStore = $cookieStoreInResponse->remove($this->config->cookieName); // But clear() method clears cookies in the object (not immutable). $cookieStoreInResponse->clear(); @@ -924,7 +939,7 @@ protected function startSession() */ protected function setCookie() { - $expiration = $this->sessionExpiration === 0 ? 0 : Time::now()->getTimestamp() + $this->sessionExpiration; + $expiration = $this->config->expiration === 0 ? 0 : Time::now()->getTimestamp() + $this->config->expiration; $this->cookie = $this->cookie->withValue(session_id())->withExpires($expiration); $response = Services::response(); diff --git a/system/Test/Mock/MockSession.php b/system/Test/Mock/MockSession.php index f5290f26525d..9f558e1034ad 100644 --- a/system/Test/Mock/MockSession.php +++ b/system/Test/Mock/MockSession.php @@ -57,7 +57,7 @@ protected function startSession() */ protected function setCookie() { - $expiration = $this->sessionExpiration === 0 ? 0 : Time::now()->getTimestamp() + $this->sessionExpiration; + $expiration = $this->config->expiration === 0 ? 0 : Time::now()->getTimestamp() + $this->config->expiration; $this->cookie = $this->cookie->withValue(session_id())->withExpires($expiration); $this->cookies[] = $this->cookie; From d58f0983d47c1005218a91713e5f03ecd0068b63 Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 21 Dec 2022 15:50:28 +0900 Subject: [PATCH 3/3] docs: fix @var SessionConfig is required. --- system/Commands/Generators/MigrationGenerator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/Commands/Generators/MigrationGenerator.php b/system/Commands/Generators/MigrationGenerator.php index a47e34196063..07a74c46b53e 100644 --- a/system/Commands/Generators/MigrationGenerator.php +++ b/system/Commands/Generators/MigrationGenerator.php @@ -107,7 +107,7 @@ protected function prepare(string $class): string $data['DBGroup'] = is_string($DBGroup) ? $DBGroup : 'default'; $data['DBDriver'] = config('Database')->{$data['DBGroup']}['DBDriver']; - /** @var SessionConfig|null $session */ + /** @var SessionConfig $session */ $session = config('Session'); $data['matchIP'] = $session->matchIP;