From 1408908c845adbdd0dd5a1f63849844142c3aff1 Mon Sep 17 00:00:00 2001 From: nupplaPhil Date: Wed, 11 Dec 2019 20:30:31 +0100 Subject: [PATCH] Refactor Session Handling (make it more simple & handler are now handler again) --- src/Core/Session.php | 35 ++++----- src/Core/Session/AbstractSession.php | 76 ++++++++++++++++++++ src/Core/Session/{ => Handler}/Cache.php | 12 +--- src/Core/Session/{ => Handler}/Database.php | 14 ++-- src/Core/Session/ISession.php | 10 +-- src/Core/Session/Memory.php | 12 +--- src/Core/Session/Native.php | 78 +++------------------ src/Factory/SessionFactory.php | 36 +++++----- src/Model/User/Cookie.php | 4 +- src/Module/Logout.php | 2 +- tests/functional/DependencyCheckTest.php | 4 ++ 11 files changed, 143 insertions(+), 140 deletions(-) create mode 100644 src/Core/Session/AbstractSession.php rename src/Core/Session/{ => Handler}/Cache.php (82%) rename src/Core/Session/{ => Handler}/Database.php (85%) diff --git a/src/Core/Session.php b/src/Core/Session.php index ef26cd929e..0557ce81b7 100644 --- a/src/Core/Session.php +++ b/src/Core/Session.php @@ -59,11 +59,14 @@ class Session extends BaseObject */ public static function getRemoteContactID($uid) { - if (empty($_SESSION['remote'][$uid])) { + /** @var ISession $session */ + $session = self::getClass(ISession::class); + + if (empty($session->get('remote')[$uid])) { return false; } - return $_SESSION['remote'][$uid]; + return $session->get('remote')[$uid]; } /** @@ -74,11 +77,14 @@ class Session extends BaseObject */ public static function getUserIDForVisitorContactID($cid) { - if (empty($_SESSION['remote'])) { + /** @var ISession $session */ + $session = self::getClass(ISession::class); + + if (empty($session->get('remote'))) { return false; } - return array_search($cid, $_SESSION['remote']); + return array_search($cid, $session->get('remote')); } /** @@ -88,15 +94,18 @@ class Session extends BaseObject */ public static function setVisitorsContacts() { - $_SESSION['remote'] = []; + /** @var ISession $session */ + $session = self::getClass(ISession::class); - $remote_contacts = DBA::select('contact', ['id', 'uid'], ['nurl' => Strings::normaliseLink($_SESSION['my_url']), 'rel' => [Contact::FOLLOWER, Contact::FRIEND], 'self' => false]); + $session->set('remote', []); + + $remote_contacts = DBA::select('contact', ['id', 'uid'], ['nurl' => Strings::normaliseLink($session->get('my_url')), 'rel' => [Contact::FOLLOWER, Contact::FRIEND], 'self' => false]); while ($contact = DBA::fetch($remote_contacts)) { if (($contact['uid'] == 0) || Contact::isBlockedByUser($contact['id'], $contact['uid'])) { continue; } - $_SESSION['remote'][$contact['uid']] = $contact['id']; + $session->set('remote', [$contact['uid'] => $contact['id']]); } DBA::close($remote_contacts); } @@ -108,15 +117,9 @@ class Session extends BaseObject */ public static function isAuthenticated() { - if (empty($_SESSION['authenticated'])) { - return false; - } + /** @var ISession $session */ + $session = self::getClass(ISession::class); - return $_SESSION['authenticated']; - } - - public static function delete() - { - self::getClass(ISession::class)->delete(); + return $session->get('authenticated', false); } } diff --git a/src/Core/Session/AbstractSession.php b/src/Core/Session/AbstractSession.php new file mode 100644 index 0000000000..58649a6bd8 --- /dev/null +++ b/src/Core/Session/AbstractSession.php @@ -0,0 +1,76 @@ +cookie = $cookie; + } + + /** + * {@inheritDoc} + */ + public function start() + { + return $this; + } + + /** + * {@inheritDoc}} + */ + public function exists(string $name) + { + return isset($_SESSION[$name]); + } + + /** + * {@inheritDoc} + */ + public function get(string $name, $defaults = null) + { + return $_SESSION[$name] ?? $defaults; + } + + /** + * {@inheritDoc} + */ + public function set(string $name, $value) + { + $_SESSION[$name] = $value; + } + + /** + * {@inheritDoc} + */ + public function setMultiple(array $values) + { + $_SESSION = $values + $_SESSION; + } + + /** + * {@inheritDoc} + */ + public function remove(string $name) + { + unset($_SESSION[$name]); + } + + /** + * {@inheritDoc} + */ + public function clear() + { + $_SESSION = []; + } +} diff --git a/src/Core/Session/Cache.php b/src/Core/Session/Handler/Cache.php similarity index 82% rename from src/Core/Session/Cache.php rename to src/Core/Session/Handler/Cache.php index f5dee0fab8..ecd266adbf 100644 --- a/src/Core/Session/Cache.php +++ b/src/Core/Session/Handler/Cache.php @@ -1,11 +1,9 @@ */ -final class Cache extends Native implements SessionHandlerInterface +final class Cache implements SessionHandlerInterface { /** @var ICache */ private $cache; @@ -23,15 +21,11 @@ final class Cache extends Native implements SessionHandlerInterface /** @var array The $_SERVER array */ private $server; - public function __construct(Configuration $config, Cookie $cookie, ICache $cache, LoggerInterface $logger, array $server) + public function __construct(ICache $cache, LoggerInterface $logger, array $server) { - parent::__construct($config, $cookie); - $this->cache = $cache; $this->logger = $logger; $this->server = $server; - - session_set_save_handler($this); } public function open($save_path, $session_name) diff --git a/src/Core/Session/Database.php b/src/Core/Session/Handler/Database.php similarity index 85% rename from src/Core/Session/Database.php rename to src/Core/Session/Handler/Database.php index 1051100229..e8151dddc8 100644 --- a/src/Core/Session/Database.php +++ b/src/Core/Session/Handler/Database.php @@ -1,11 +1,9 @@ */ -final class Database extends Native implements SessionHandlerInterface +final class Database implements SessionHandlerInterface { /** @var DBA */ private $dba; @@ -26,19 +24,15 @@ final class Database extends Native implements SessionHandlerInterface /** * DatabaseSessionHandler constructor. * - * @param Database $dba + * @param DBA $dba * @param LoggerInterface $logger * @param array $server */ - public function __construct(Configuration $config, Cookie $cookie, DBA $dba, LoggerInterface $logger, array $server) + public function __construct(DBA $dba, LoggerInterface $logger, array $server) { - parent::__construct($config, $cookie); - $this->dba = $dba; $this->logger = $logger; $this->server = $server; - - session_set_save_handler($this); } public function open($save_path, $session_name) diff --git a/src/Core/Session/ISession.php b/src/Core/Session/ISession.php index a2046e915c..dbc7fd85b4 100644 --- a/src/Core/Session/ISession.php +++ b/src/Core/Session/ISession.php @@ -29,7 +29,8 @@ interface ISession * Handle the case where session_start() hasn't been called and the super global isn't available. * * @param string $name - * @param mixed $defaults + * @param mixed $defaults + * * @return mixed */ public function get(string $name, $defaults = null); @@ -39,7 +40,7 @@ interface ISession * Overrides value of existing key. * * @param string $name - * @param mixed $value + * @param mixed $value */ public function set(string $name, $value); @@ -63,9 +64,4 @@ interface ISession * Clears the current session array */ public function clear(); - - /** - * Kills the "Friendica" cookie and all session data - */ - public function delete(); } diff --git a/src/Core/Session/Memory.php b/src/Core/Session/Memory.php index f8d02b5538..de03ea15c4 100644 --- a/src/Core/Session/Memory.php +++ b/src/Core/Session/Memory.php @@ -2,7 +2,6 @@ namespace Friendica\Core\Session; -use Friendica\Core\Config\Configuration; use Friendica\Model\User\Cookie; /** @@ -10,19 +9,14 @@ use Friendica\Model\User\Cookie; * * @todo after replacing the last direct $_SESSION call, use a internal array instead of the global variable */ -final class Memory extends Native +final class Memory extends AbstractSession implements ISession { - public function __construct(Configuration $config, Cookie $cookie) + public function __construct(Cookie $cookie) { - $this->cookie = $cookie; - } + parent::__construct($cookie); - public function start() - { // Backward compatibility until all Session variables are replaced // with the Session class $_SESSION = []; - $this->clear(); - return $this; } } diff --git a/src/Core/Session/Native.php b/src/Core/Session/Native.php index b7f992b274..997ac43f68 100644 --- a/src/Core/Session/Native.php +++ b/src/Core/Session/Native.php @@ -2,29 +2,30 @@ namespace Friendica\Core\Session; -use Friendica\Core\Config\Configuration; use Friendica\App; use Friendica\Model\User\Cookie; +use SessionHandlerInterface; /** - * The native Session class which uses the PHP internal Session function + * The native Session class which uses the PHP internal Session functions */ -class Native implements ISession +final class Native extends AbstractSession implements ISession { - /** @var Cookie */ - protected $cookie; - - public function __construct(Configuration $config, Cookie $cookie) + public function __construct(App\BaseURL $baseURL, Cookie $cookie, SessionHandlerInterface $handler = null) { + parent::__construct($cookie); + ini_set('session.gc_probability', 50); ini_set('session.use_only_cookies', 1); - ini_set('session.cookie_httponly', 1); + ini_set('session.cookie_httponly', (int)Cookie::HTTPONLY); - if ($config->get('system', 'ssl_policy') == App\BaseURL::SSL_POLICY_FULL) { + if ($baseURL->getSSLPolicy() == App\BaseURL::SSL_POLICY_FULL) { ini_set('session.cookie_secure', 1); } - $this->cookie = $cookie; + if (isset($handler)) { + session_set_save_handler($handler); + } } /** @@ -35,61 +36,4 @@ class Native implements ISession session_start(); return $this; } - - /** - * {@inheritDoc}} - */ - public function exists(string $name) - { - return isset($_SESSION[$name]); - } - - /** - * {@inheritDoc} - */ - public function get(string $name, $defaults = null) - { - return $_SESSION[$name] ?? $defaults; - } - - /** - * {@inheritDoc} - */ - public function set(string $name, $value) - { - $_SESSION[$name] = $value; - } - - /** - * {@inheritDoc} - */ - public function setMultiple(array $values) - { - $_SESSION = $values + $_SESSION; - } - - /** - * {@inheritDoc} - */ - public function remove(string $name) - { - unset($_SESSION[$name]); - } - - /** - * {@inheritDoc} - */ - public function clear() - { - $_SESSION = []; - } - - /** - * @brief Kills the "Friendica" cookie and all session data - */ - public function delete() - { - $this->cookie->clear(); - $_SESSION = []; - } } diff --git a/src/Factory/SessionFactory.php b/src/Factory/SessionFactory.php index f4268a2b95..017eab0adb 100644 --- a/src/Factory/SessionFactory.php +++ b/src/Factory/SessionFactory.php @@ -19,18 +19,17 @@ use Psr\Log\LoggerInterface; class SessionFactory { /** @var string The plain, PHP internal session management */ - const INTERNAL = 'native'; + const HANDLER_NATIVE = 'native'; /** @var string Using the database for session management */ - const DATABASE = 'database'; + const HANDLER_DATABASE = 'database'; /** @var string Using the cache for session management */ - const CACHE = 'cache'; - /** @var string A temporary cached session */ - const MEMORY = 'memory'; - /** @var string The default type for Session management in case of no config */ - const DEFAULT = self::DATABASE; + const HANDLER_CACHE = 'cache'; + + const HANDLER_DEFAULT = self::HANDLER_DATABASE; /** * @param App\Mode $mode + * @param App\BaseURL $baseURL * @param Configuration $config * @param Cookie $cookie * @param Database $dba @@ -40,34 +39,33 @@ class SessionFactory * * @return Session\ISession */ - public function createSession(App\Mode $mode, Configuration $config, Cookie $cookie, Database $dba, ICache $cache, LoggerInterface $logger, Profiler $profiler, array $server = []) + public function createSession(App\Mode $mode, App\BaseURL $baseURL, Configuration $config, Cookie $cookie, Database $dba, ICache $cache, LoggerInterface $logger, Profiler $profiler, array $server = []) { $stamp1 = microtime(true); $session = null; try { if ($mode->isInstall() || $mode->isBackend()) { - $session = new Session\Memory($config, $cookie); + $session = new Session\Memory($cookie); } else { - $session_handler = $config->get('system', 'session_handler', self::DEFAULT); + $session_handler = $config->get('system', 'session_handler', self::HANDLER_DEFAULT); + $handler = null; switch ($session_handler) { - case self::INTERNAL: - $session = new Session\Native($config, $cookie); + case self::HANDLER_DATABASE: + $handler = new Session\Handler\Database($dba, $logger, $server); break; - case self::DATABASE: - default: - $session = new Session\Database($config, $cookie, $dba, $logger, $server); - break; - case self::CACHE: + case self::HANDLER_CACHE: // In case we're using the db as cache driver, use the native db session, not the cache if ($config->get('system', 'cache_driver') === Cache::TYPE_DATABASE) { - $session = new Session\Database($config, $cookie, $dba, $logger, $server); + $handler = new Session\Handler\Database($dba, $logger, $server); } else { - $session = new Session\Cache($config, $cookie, $cache, $logger, $server); + $handler = new Session\Handler\Cache($cache, $logger, $server); } break; } + + $session = new Session\Native($baseURL, $cookie, $handler); } } finally { $profiler->saveTimestamp($stamp1, 'parser', System::callstack()); diff --git a/src/Model/User/Cookie.php b/src/Model/User/Cookie.php index f85d818686..0fc90222f1 100644 --- a/src/Model/User/Cookie.php +++ b/src/Model/User/Cookie.php @@ -32,13 +32,13 @@ class Cookie /** @var array The $_COOKIE array */ private $cookie; - public function __construct(Configuration $config, array $server = [], array $cookie = []) + public function __construct(Configuration $config, App\BaseURL $baseURL, array $server = [], array $cookie = []) { if (!empty($server['REMOTE_ADDR'])) { $this->remoteAddr = $server['REMOTE_ADDR']; } - $this->sslEnabled = $config->get('system', 'ssl_policy') === App\BaseURL::SSL_POLICY_FULL; + $this->sslEnabled = $baseURL->getSSLPolicy() === App\BaseURL::SSL_POLICY_FULL; $this->sitePrivateKey = $config->get('system', 'site_prvkey'); $authCookieDays = $config->get('system', 'auth_cookie_lifetime', diff --git a/src/Module/Logout.php b/src/Module/Logout.php index 89910107d0..9e6c674b35 100644 --- a/src/Module/Logout.php +++ b/src/Module/Logout.php @@ -33,7 +33,7 @@ class Logout extends BaseModule } Hook::callAll("logging_out"); - Session::delete(); + Session::clear(); if ($visitor_home) { System::externalRedirect($visitor_home); diff --git a/tests/functional/DependencyCheckTest.php b/tests/functional/DependencyCheckTest.php index 6de2dc6479..336a34c344 100644 --- a/tests/functional/DependencyCheckTest.php +++ b/tests/functional/DependencyCheckTest.php @@ -133,6 +133,10 @@ class dependencyCheck extends TestCase public function testDevLogger() { + /** @var Configuration $config */ + $config = $this->dice->create(Configuration::class); + $config->set('system', 'dlogfile', $this->root->url() . '/friendica.log'); + /** @var LoggerInterface $logger */ $logger = $this->dice->create('$devLogger', ['dev']);