diff --git a/.upgradenotes/MDL-66151-2024090301594788.yml b/.upgradenotes/MDL-66151-2024090301594788.yml new file mode 100644 index 00000000000..c9f58e93037 --- /dev/null +++ b/.upgradenotes/MDL-66151-2024090301594788.yml @@ -0,0 +1,15 @@ +issueNumber: MDL-66151 +notes: + core_role: + - message: | + Move all session management to the \core\session\manager class. + This removes the dependancy to use the "sessions" table. + Session management plugins (like redis) now need to inherit + the base \core\session\handler class which implements + SessionHandlerInterface and override methods as required. + The following methods in \core\session\manager have been deprecated: + * kill_all_sessions use destroy_all instead + * kill_session use destroy instead + * kill_sessions_for_auth_plugin use destroy_by_auth_plugin instead + * kill_user_sessions use destroy_user_sessions instead + type: improved diff --git a/admin/tool/mfa/factor/email/email.php b/admin/tool/mfa/factor/email/email.php index be0517a85bf..66fdbf0b3c2 100644 --- a/admin/tool/mfa/factor/email/email.php +++ b/admin/tool/mfa/factor/email/email.php @@ -79,7 +79,7 @@ if ($form->is_cancelled()) { $DB->set_field('tool_mfa', 'revoked', 1, ['userid' => $user->id, 'factor' => 'email']); // Remotely logout all sessions for user. - $manager = \core\session\manager::kill_user_sessions($instance->userid); + \core\session\manager::destroy_user_sessions($instance->userid); // Log event. $ip = $instance->createdfromip; diff --git a/admin/tool/uploaduser/classes/process.php b/admin/tool/uploaduser/classes/process.php index a13013f59a4..562e5895629 100644 --- a/admin/tool/uploaduser/classes/process.php +++ b/admin/tool/uploaduser/classes/process.php @@ -931,7 +931,7 @@ class process { } if ($dologout) { - \core\session\manager::kill_user_sessions($existinguser->id); + \core\session\manager::destroy_user_sessions($existinguser->id); } } else { diff --git a/admin/user.php b/admin/user.php index 266377600f3..43925756ae7 100644 --- a/admin/user.php +++ b/admin/user.php @@ -131,7 +131,7 @@ if (!is_siteadmin($user) and $USER->id != $user->id and $user->suspended != 1) { $user->suspended = 1; // Force logout. - \core\session\manager::kill_user_sessions($user->id); + \core\session\manager::destroy_user_sessions($user->id); user_update_user($user, false); } } diff --git a/auth/ldap/auth.php b/auth/ldap/auth.php index 927968ccd87..c423e0166d0 100644 --- a/auth/ldap/auth.php +++ b/auth/ldap/auth.php @@ -839,7 +839,7 @@ class auth_plugin_ldap extends auth_plugin_base { $updateuser->suspended = 1; user_update_user($updateuser, false); echo "\t"; print_string('auth_dbsuspenduser', 'auth_db', array('name'=>$user->username, 'id'=>$user->id)); echo "\n"; - \core\session\manager::kill_user_sessions($user->id); + \core\session\manager::destroy_user_sessions($user->id); } } else { print_string('nouserentriestoremove', 'auth_ldap'); diff --git a/auth/mnet/auth.php b/auth/mnet/auth.php index 676368ebc60..f5060d5ab6b 100644 --- a/auth/mnet/auth.php +++ b/auth/mnet/auth.php @@ -868,7 +868,7 @@ class auth_plugin_mnet extends auth_plugin_base { array('useragent'=>$useragent, 'userid'=>$userid)); if (isset($remoteclient) && isset($remoteclient->id)) { - \core\session\manager::kill_user_sessions($userid); + \core\session\manager::destroy_user_sessions($userid); } return $returnstring; } @@ -888,7 +888,7 @@ class auth_plugin_mnet extends auth_plugin_base { $session = $DB->get_record('mnet_session', array('username'=>$username, 'mnethostid'=>$remoteclient->id, 'useragent'=>$useragent)); $DB->delete_records('mnet_session', array('username'=>$username, 'mnethostid'=>$remoteclient->id, 'useragent'=>$useragent)); if (false != $session) { - \core\session\manager::kill_session($session->session_id); + \core\session\manager::destroy($session->session_id); return true; } return false; @@ -905,7 +905,7 @@ class auth_plugin_mnet extends auth_plugin_base { global $CFG; if (is_array($sessionArray)) { while($session = array_pop($sessionArray)) { - \core\session\manager::kill_session($session->session_id); + \core\session\manager::destroy($session->session_id); } return true; } diff --git a/auth/shibboleth/classes/helper.php b/auth/shibboleth/classes/helper.php index 297146fb221..4be2c3d68a8 100644 --- a/auth/shibboleth/classes/helper.php +++ b/auth/shibboleth/classes/helper.php @@ -98,7 +98,7 @@ class helper { // If there is a match, kill the session. if ($usersession['SESSION']->shibboleth_session_id == trim($spsessionid)) { // Delete this user's sessions. - \core\session\manager::kill_user_sessions($session->userid); + \core\session\manager::destroy_user_sessions($session->userid); } } } diff --git a/cache/stores/redis/lib.php b/cache/stores/redis/lib.php index 0c96afe220d..28e5a8833e6 100644 --- a/cache/stores/redis/lib.php +++ b/cache/stores/redis/lib.php @@ -604,7 +604,8 @@ class cachestore_redis extends store implements * @return bool True if the lock was acquired, false if it was not. */ public function acquire_lock($key, $ownerid) { - $timelimit = time() + $this->lockwait; + $clock = \core\di::get(\core\clock::class); + $timelimit = $clock->time() + $this->lockwait; do { // If the key doesn't already exist, grab it and return true. if ($this->redis->setnx($key, $ownerid)) { @@ -620,7 +621,7 @@ class cachestore_redis extends store implements } // Wait 1 second then retry. sleep(1); - } while (time() < $timelimit); + } while ($clock->time() < $timelimit); return false; } diff --git a/lib/behat/classes/behat_session_trait.php b/lib/behat/classes/behat_session_trait.php index 7ec03499b40..830b7b46454 100644 --- a/lib/behat/classes/behat_session_trait.php +++ b/lib/behat/classes/behat_session_trait.php @@ -1163,11 +1163,11 @@ EOF; if (empty($sid)) { throw new coding_exception('failed to get moodle session'); } - $userid = $DB->get_field('sessions', 'userid', ['sid' => $sid]); - if (empty($userid)) { - throw new coding_exception('failed to get user from seession id '.$sid); + $session = \core\session\manager::get_session_by_sid($sid); + if (empty($session->userid)) { + throw new coding_exception('failed to get user from session id: '.$sid); } - return $DB->get_record('user', ['id' => $userid]); + return $DB->get_record('user', ['id' => $session->userid]); } /** diff --git a/lib/classes/plugininfo/auth.php b/lib/classes/plugininfo/auth.php index 1c7a4e71be6..f85013d569e 100644 --- a/lib/classes/plugininfo/auth.php +++ b/lib/classes/plugininfo/auth.php @@ -87,6 +87,7 @@ class auth extends base { $new = implode(',', array_flip($plugins)); add_to_config_log('auth', $CFG->auth, $new, 'core'); set_config('auth', $new); + \core\session\manager::destroy_by_auth_plugin($pluginname); // Remove stale sessions. \core\session\manager::gc(); // Reset caches. @@ -160,6 +161,7 @@ class auth extends base { $value = implode(',', $auths); add_to_config_log('auth', $CFG->auth, $value, 'core'); set_config('auth', $value); + \core\session\manager::destroy_by_auth_plugin($this->name); } if (!empty($CFG->registerauth) and $CFG->registerauth === $this->name) { diff --git a/lib/classes/session/database.php b/lib/classes/session/database.php index 6b15884018b..1c8b68de8c2 100644 --- a/lib/classes/session/database.php +++ b/lib/classes/session/database.php @@ -14,14 +14,6 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -/** - * Database based session handler. - * - * @package core - * @copyright 2013 Petr Skoda {@link http://skodak.org} - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - namespace core\session; use SessionHandlerInterface; @@ -34,7 +26,8 @@ use SessionHandlerInterface; * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class database extends handler implements SessionHandlerInterface { - /** @var \stdClass $record session record */ + + /** @var int $record session record */ protected $recordid = null; /** @var \moodle_database $database session database */ @@ -62,9 +55,7 @@ class database extends handler implements SessionHandlerInterface { } } - /** - * Init session handler. - */ + #[\Override] public function init() { if (!$this->database->session_lock_supported()) { throw new exception('sessionhandlerproblem', 'error', '', null, 'Database does not support session locking'); @@ -76,35 +67,36 @@ class database extends handler implements SessionHandlerInterface { } } - /** - * Check the backend contains data for this session id. - * - * Note: this is intended to be called from manager::session_exists() only. - * - * @param string $sid - * @return bool true if session found. - */ + #[\Override] public function session_exists($sid) { // It was already checked in the calling code that the record in sessions table exists. return true; } - /** - * Kill all active sessions, the core sessions table is - * purged afterwards. - */ - public function kill_all_sessions() { - // Nothing to do, the sessions table is cleared from core. - return; - } + #[\Override] + public function destroy(string $id): bool { + if (!$session = $this->get_session_by_sid($id)) { + if ($id == session_id()) { + $this->recordid = null; + $this->lasthash = null; + } + return true; + } - /** - * Kill one session, the session record is removed afterwards. - * @param string $sid - */ - public function kill_session($sid) { - // Nothing to do, the sessions table is purged afterwards. - return; + if ($this->recordid && ($session->id == $this->recordid)) { + try { + $this->database->release_session_lock($this->recordid); + } catch (\Exception $ex) { + // Log and ignore any problems. + mtrace('Failed to release session lock: '.$ex->getMessage()); + } + $this->recordid = null; + $this->lasthash = null; + } + + $this->database->delete_records('sessions', ['id' => $session->id]); + + return true; } /** @@ -151,7 +143,7 @@ class database extends handler implements SessionHandlerInterface { */ public function read(string $sid): string|false { try { - if (!$record = $this->database->get_record('sessions', array('sid'=>$sid), 'id')) { + if (!$record = $this->get_session_by_sid($sid)) { // Let's cheat and skip locking if this is the first access, // do not create the record here, let the manager do it after session init. $this->failed = false; @@ -252,56 +244,4 @@ class database extends handler implements SessionHandlerInterface { return true; } - /** - * Destroy session handler. - * - * {@see http://php.net/manual/en/function.session-set-save-handler.php} - * - * @param string $id - * @return bool success - */ - public function destroy(string $id): bool { - if (!$session = $this->database->get_record('sessions', ['sid' => $id], 'id, sid')) { - if ($id == session_id()) { - $this->recordid = null; - $this->lasthash = null; - } - return true; - } - - if ($this->recordid && ($session->id == $this->recordid)) { - try { - $this->database->release_session_lock($this->recordid); - } catch (\Exception $ex) { - // Ignore problems. - } - $this->recordid = null; - $this->lasthash = null; - } - - $this->database->delete_records('sessions', ['id' => $session->id]); - - return true; - } - - /** - * GC session handler. - * - * {@see http://php.net/manual/en/function.session-set-save-handler.php} - * - * @param int $max_lifetime moodle uses special timeout rules - * @return bool success - */ - // phpcs:ignore moodle.NamingConventions.ValidVariableName.VariableNameUnderscore - public function gc(int $max_lifetime): int|false { - // This should do something only if cron is not running properly... - if (!$stalelifetime = ini_get('session.gc_maxlifetime')) { - return false; - } - $params = ['purgebefore' => (time() - $stalelifetime)]; - $count = $this->database->count_records_select('sessions', 'userid = 0 AND timemodified < :purgebefore', $params); - $this->database->delete_records_select('sessions', 'userid = 0 AND timemodified < :purgebefore', $params); - - return $count; - } } diff --git a/lib/classes/session/file.php b/lib/classes/session/file.php index b454e0e7b90..b84e543a093 100644 --- a/lib/classes/session/file.php +++ b/lib/classes/session/file.php @@ -14,18 +14,8 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -/** - * File based session handler. - * - * @package core - * @copyright 2013 Petr Skoda {@link http://skodak.org} - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - namespace core\session; -defined('MOODLE_INTERNAL') || die(); - /** * File based session handler. * @@ -34,6 +24,7 @@ defined('MOODLE_INTERNAL') || die(); * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class file extends handler { + /** @var string session dir */ protected $sessiondir; @@ -50,9 +41,7 @@ class file extends handler { } } - /** - * Init session handler. - */ + #[\Override] public function init() { if (preg_match('/^[0-9]+;/', $this->sessiondir)) { throw new exception('sessionhandlerproblem', 'error', '', null, 'Multilevel session directories are not supported'); @@ -75,14 +64,7 @@ class file extends handler { ini_set('session.save_path', $this->sessiondir); } - /** - * Check the backend contains data for this session id. - * - * Note: this is intended to be called from manager::session_exists() only. - * - * @param string $sid - * @return bool true if session found. - */ + #[\Override] public function session_exists($sid) { $sid = clean_param($sid, PARAM_FILE); if (!$sid) { @@ -92,30 +74,28 @@ class file extends handler { return file_exists($sessionfile); } - /** - * Kill all active sessions, the core sessions table is - * purged afterwards. - */ - public function kill_all_sessions() { + #[\Override] + public function destroy_all(): bool { if (is_dir($this->sessiondir)) { foreach (glob("$this->sessiondir/sess_*") as $filename) { @unlink($filename); } } + + return true; } - /** - * Kill one session, the session record is removed afterwards. - * @param string $sid - */ - public function kill_session($sid) { - $sid = clean_param($sid, PARAM_FILE); + #[\Override] + public function destroy(string $id): bool { + $sid = clean_param($id, PARAM_FILE); if (!$sid) { - return; + return false; } $sessionfile = "$this->sessiondir/sess_$sid"; if (file_exists($sessionfile)) { @unlink($sessionfile); } + + return true; } } diff --git a/lib/classes/session/handler.php b/lib/classes/session/handler.php index 07cec66d30b..1736bd0c47f 100644 --- a/lib/classes/session/handler.php +++ b/lib/classes/session/handler.php @@ -14,17 +14,11 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -/** - * Session handler base. - * - * @package core - * @copyright 2013 Petr Skoda {@link http://skodak.org} - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - namespace core\session; -defined('MOODLE_INTERNAL') || die(); +use core\clock; +use core\di; +use stdClass; /** * Session handler base. @@ -79,6 +73,220 @@ abstract class handler { $this->requireswritelock = $requireswritelock; } + /** + * Returns all session records. + * + * @return \Iterator + */ + public function get_all_sessions(): \Iterator { + global $DB; + + $rs = $DB->get_recordset('sessions'); + foreach ($rs as $row) { + yield $row; + } + $rs->close(); + } + + /** + * Returns a single session record for this session id. + * + * @param string $sid + * @return stdClass + */ + public function get_session_by_sid(string $sid): stdClass { + global $DB; + + return $DB->get_record('sessions', ['sid' => $sid]) ?: new stdClass(); + } + + /** + * Returns all the session records for this user id. + * + * @param int $userid + * @return array + */ + public function get_sessions_by_userid(int $userid): array { + global $DB; + + return $DB->get_records('sessions', ['userid' => $userid]); + } + + /** + * Insert new empty session record. + * + * @param int $userid + * @return stdClass the new record + */ + public function add_session(int $userid): stdClass { + global $DB; + + $record = new stdClass(); + $record->state = 0; + $record->sid = session_id(); + $record->sessdata = null; + $record->userid = $userid; + $record->timecreated = $record->timemodified = di::get(clock::class)->time(); + $record->firstip = $record->lastip = getremoteaddr(); + + $record->id = $DB->insert_record('sessions', $record); + + return $record; + } + + /** + * Update a session record. + * + * @param stdClass $record + * @return bool + */ + public function update_session(stdClass $record): bool { + global $DB; + + if (!isset($record->id) && isset($record->sid)) { + $record->id = $DB->get_field('sessions', 'id', ['sid' => $record->sid]); + } + + return $DB->update_record('sessions', $record); + } + + /** + * Destroy a specific session and delete this session record for this session id. + * + * @param string $id session id + * @return bool + */ + public function destroy(string $id): bool { + global $DB; + + return $DB->delete_records('sessions', ['sid' => $id]); + } + + /** + * Destroy all sessions, and delete all the session data. + * + * @return bool + */ + public function destroy_all(): bool { + global $DB; + + return $DB->delete_records('sessions'); + } + + /** + * Clean up expired sessions. + * + * @param int $purgebefore Sessions that have not updated for the last purgebefore timestamp will be removed. + * @param int $userid + */ + protected function destroy_expired_user_sessions(int $purgebefore, int $userid): void { + $sessions = $this->get_sessions_by_userid($userid); + foreach ($sessions as $session) { + if ($session->timemodified < $purgebefore) { + $this->destroy($session->sid); + } + } + } + + /** + * Clean up all expired sessions. + * + * @param int $purgebefore + */ + protected function destroy_all_expired_sessions(int $purgebefore): void { + global $DB, $CFG; + + $authsequence = get_enabled_auth_plugins(); + $authsequence = array_flip($authsequence); + unset($authsequence['nologin']); // No login means user cannot login. + $authsequence = array_flip($authsequence); + $authplugins = []; + foreach ($authsequence as $authname) { + $authplugins[$authname] = get_auth_plugin($authname); + } + $sql = "SELECT u.*, s.sid, s.timecreated AS s_timecreated, s.timemodified AS s_timemodified + FROM {user} u + JOIN {sessions} s ON s.userid = u.id + WHERE s.timemodified < :purgebefore AND u.id <> :guestid"; + $params = ['purgebefore' => $purgebefore, 'guestid' => $CFG->siteguest]; + + $rs = $DB->get_recordset_sql($sql, $params); + foreach ($rs as $user) { + foreach ($authplugins as $authplugin) { + if ($authplugin->ignore_timeout_hook($user, $user->sid, $user->s_timecreated, $user->s_timemodified)) { + continue 2; + } + } + $this->destroy($user->sid); + } + $rs->close(); + } + + /** + * Destroy all sessions for a given plugin. + * Typically used when a plugin is disabled or uninstalled, so all sessions (users) for that plugin are logged out. + * + * @param string $pluginname Auth plugin name. + */ + public function destroy_by_auth_plugin(string $pluginname): void { + global $DB; + + $rs = $DB->get_recordset('user', ['auth' => $pluginname], 'id ASC', 'id'); + foreach ($rs as $user) { + $sessions = $this->get_sessions_by_userid($user->id); + foreach ($sessions as $session) { + $this->destroy($session->sid); + } + } + $rs->close(); + } + + // phpcs:disable moodle.NamingConventions.ValidVariableName.VariableNameUnderscore + /** + * Periodic timed-out session cleanup. + * + * @param int $max_lifetime Sessions that have not updated for the last max_lifetime seconds will be removed. + * @return int|false Number of deleted sessions or false if an error occurred. + */ + public function gc(int $max_lifetime = 0): int|false { + global $CFG; + + // This may take a long time. + \core_php_time_limit::raise(); + + if ($max_lifetime === 0) { + $max_lifetime = $CFG->sessiontimeout; + } + + try { + // Delete expired sessions for guest user account, give them larger timeout, there is no security risk here. + $purgebefore = di::get(clock::class)->time() - ($max_lifetime * 5); + $this->destroy_expired_user_sessions($purgebefore, $CFG->siteguest); + + // Delete expired sessions for userid = 0 (not logged in), better kill them asap to release memory. + $purgebefore = di::get(clock::class)->time() - $max_lifetime; + $this->destroy_expired_user_sessions($purgebefore, 0); + + // Clean up expired sessions for real users only. + $this->destroy_all_expired_sessions($purgebefore); + + // Cleanup leftovers from the first browser access because it may set multiple cookies and then use only one. + $purgebefore = di::get(clock::class)->time() - (60 * 3); + $sessions = $this->get_sessions_by_userid(0); + foreach ($sessions as $session) { + if ($session->timemodified == $session->timecreated && $session->timemodified < $purgebefore) { + $this->destroy($session->sid); + } + } + + } catch (\Exception $ex) { + debugging('Error gc-ing sessions: '.$ex->getMessage(), DEBUG_NORMAL, $ex->getTrace()); + } + + return 0; + } + // phpcs:enable + /** * Has this session been opened with a writelock? Your handler should call this during * start() if you support read-only sessions. @@ -102,16 +310,4 @@ abstract class handler { * @return bool true if session found. */ abstract public function session_exists($sid); - - /** - * Kill all active sessions, the core sessions table is - * purged afterwards. - */ - abstract public function kill_all_sessions(); - - /** - * Kill one session, the session record is removed afterwards. - * @param string $sid - */ - abstract public function kill_session($sid); } diff --git a/lib/classes/session/manager.php b/lib/classes/session/manager.php index 4ce12f52ac8..7070ec9c42d 100644 --- a/lib/classes/session/manager.php +++ b/lib/classes/session/manager.php @@ -14,18 +14,10 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -/** - * Session manager class. - * - * @package core - * @copyright 2013 Petr Skoda {@link http://skodak.org} - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - namespace core\session; -defined('MOODLE_INTERNAL') || die(); - +use core\clock; +use core\di; use html_writer; /** @@ -252,14 +244,14 @@ class manager { global $CFG, $DB; if (PHPUNIT_TEST) { - return '\core\session\file'; + return \core\tests\session\mock_handler::class; } else if (!empty($CFG->session_handler_class)) { return $CFG->session_handler_class; - } else if (!empty($CFG->dbsessions) and $DB->session_lock_supported()) { - return '\core\session\database'; + } else if (!empty($CFG->dbsessions) && $DB->session_lock_supported()) { + return database::class; } - return '\core\session\file'; + return file::class; } /** @@ -273,6 +265,9 @@ class manager { // Find out which handler to use. $class = self::get_handler_class(); self::$handler = new $class(); + if (!self::$handler instanceof \core\session\handler) { + throw new exception("$class must implement the \core\session\handler"); + } } /** @@ -419,7 +414,7 @@ class manager { * @param bool $newsid is this a new session in first http request? */ protected static function initialise_user_session($newsid) { - global $CFG, $DB; + global $CFG; $sid = session_id(); if (!$sid) { @@ -428,8 +423,8 @@ class manager { self::init_empty_session($newsid); return; } - - if (!$record = $DB->get_record('sessions', array('sid'=>$sid), 'id, sid, state, userid, lastip, timecreated, timemodified')) { + $record = self::get_session_by_sid($sid); + if (!isset($record->sid)) { if (!$newsid) { if (!empty($_SESSION['USER']->id)) { // This should not happen, just log it, we MUST not produce any output here! @@ -456,7 +451,7 @@ class manager { // Ignore guest and not-logged in timeouts, there is very little risk here. $timeout = false; - } else if ($record->timemodified < time() - $maxlifetime) { + } else if ($record->timemodified < di::get(clock::class)->time() - $maxlifetime) { $timeout = true; $authsequence = get_enabled_auth_plugins(); // Auths, in sequence. foreach ($authsequence as $authname) { @@ -474,11 +469,9 @@ class manager { } session_regenerate_id(true); $_SESSION = array(); - $DB->delete_records('sessions', array('id'=>$record->id)); - + self::destroy($record->sid); } else { // Update session tracking record. - $update = new \stdClass(); $updated = false; @@ -493,33 +486,34 @@ class manager { $updated = true; } + $time = di::get(clock::class)->time(); + $updatefreq = empty($CFG->session_update_timemodified_frequency) ? 20 : $CFG->session_update_timemodified_frequency; if ($record->timemodified == $record->timecreated) { // Always do first update of existing record. - $update->timemodified = $record->timemodified = time(); + $update->timemodified = $record->timemodified = $time; $updated = true; - } else if ($record->timemodified < time() - $updatefreq) { + } else if ($record->timemodified < $time - $updatefreq) { // Update the session modified flag only once every 20 seconds. - $update->timemodified = $record->timemodified = time(); + $update->timemodified = $record->timemodified = $time; $updated = true; } if ($updated && (!defined('NO_SESSION_UPDATE') || !NO_SESSION_UPDATE)) { $update->id = $record->id; - $DB->update_record('sessions', $update); + $update->userid = $record->userid; + self::$handler->update_session($update); } return; } - } else { - if ($record) { - // This happens when people switch session handlers... - session_regenerate_id(true); - $_SESSION = array(); - $DB->delete_records('sessions', array('id'=>$record->id)); - } + } else if (isset($record->sid)) { + // This happens when people switch session handlers... + session_regenerate_id(true); + $_SESSION = []; + self::destroy($record->sid); } unset($record); @@ -551,10 +545,10 @@ class manager { // Setup $USER and insert the session tracking record. if ($user) { self::set_user($user); - self::add_session_record($user->id); + self::add_session($user->id); } else { self::init_empty_session($newsid); - self::add_session_record(0); + self::add_session(0); } if ($timedout) { @@ -562,24 +556,44 @@ class manager { } } + /** + * Returns a single session record for this session id. + * + * @param string $sid + * @return \stdClass + */ + public static function get_session_by_sid(string $sid): \stdClass { + return self::$handler->get_session_by_sid($sid); + } + + /** + * Returns all the session records for this user id. + * + * @param int $userid + * @return array + */ + public static function get_sessions_by_userid(int $userid): array { + return self::$handler->get_sessions_by_userid($userid); + } + /** * Insert new empty session record. + * * @param int $userid * @return \stdClass the new record */ - protected static function add_session_record($userid) { - global $DB; - $record = new \stdClass(); - $record->state = 0; - $record->sid = session_id(); - $record->sessdata = null; - $record->userid = $userid; - $record->timecreated = $record->timemodified = time(); - $record->firstip = $record->lastip = getremoteaddr(); + public static function add_session(int $userid): \stdClass { + return self::$handler->add_session($userid); + } - $record->id = $DB->insert_record('sessions', $record); - - return $record; + /** + * Update a session record. + * + * @param \stdClass $record + * @return bool + */ + public static function update_session(\stdClass $record): bool { + return self::$handler->update_session($record); } /** @@ -619,8 +633,8 @@ class manager { $sid = session_id(); session_regenerate_id(true); - $DB->delete_records('sessions', array('sid'=>$sid)); - self::add_session_record($user->id); + self::destroy($sid); + self::add_session($user->id); // Let enrol plugins deal with new enrolments if necessary. enrol_check_plugins($user); @@ -679,9 +693,9 @@ class manager { // Write new empty session and make sure the old one is deleted. $sid = session_id(); session_regenerate_id(true); - $DB->delete_records('sessions', array('sid'=>$sid)); + self::destroy($sid); self::init_empty_session(); - self::add_session_record($_SESSION['USER']->id); // Do not use $USER here because it may not be set up yet. + self::add_session($_SESSION['USER']->id); // Do not use $USER here because it may not be set up yet. self::write_close(); } @@ -814,13 +828,14 @@ class manager { } // Note: add sessions->state checking here if it gets implemented. - if (!$record = $DB->get_record('sessions', array('sid' => $sid), 'id, userid, timemodified')) { + $record = self::get_session_by_sid($sid); + if (!isset($record->sid)) { return false; } if (empty($record->userid) or isguestuser($record->userid)) { // Ignore guest and not-logged-in timeouts, there is very little risk here. - } else if ($record->timemodified < time() - $CFG->sessiontimeout) { + } else if ($record->timemodified < di::get(clock::class)->time() - $CFG->sessiontimeout) { return false; } @@ -842,7 +857,7 @@ class manager { } // Note: add sessions->state checking here if it gets implemented. - if (!$record = $DB->get_record('sessions', array('sid' => $sid), 'id, userid, timemodified')) { + if (!$record = self::get_session_by_sid($sid)) { return ['userid' => 0, 'timeremaining' => $CFG->sessiontimeout]; } @@ -850,7 +865,10 @@ class manager { // Ignore guest and not-logged-in timeouts, there is very little risk here. return ['userid' => 0, 'timeremaining' => $CFG->sessiontimeout]; } else { - return ['userid' => $record->userid, 'timeremaining' => $CFG->sessiontimeout - (time() - $record->timemodified)]; + return [ + 'userid' => $record->userid, + 'timeremaining' => $CFG->sessiontimeout - (di::get(clock::class)->time() - $record->timemodified), + ]; } } @@ -859,64 +877,137 @@ class manager { * @param string $sid */ public static function touch_session($sid) { - global $DB; - // Timeouts depend on core sessions table only, no need to update anything in external stores. - - $sql = "UPDATE {sessions} SET timemodified = :now WHERE sid = :sid"; - $DB->execute($sql, array('now'=>time(), 'sid'=>$sid)); + self::$handler->update_session((object) [ + 'sid' => $sid, + 'timemodified' => di::get(clock::class)->time(), + ]); } /** * Terminate all sessions unconditionally. + * + * @return void + * @deprecated since Moodle 4.5 See MDL-66161 + * @todo Remove in MDL-81848 */ - public static function kill_all_sessions() { - global $DB; - - self::terminate_current(); - - self::load_handler(); - self::$handler->kill_all_sessions(); - - try { - $DB->delete_records('sessions'); - } catch (\dml_exception $ignored) { - // Do not show any warnings - might be during upgrade/installation. - } + #[\core\attribute\deprecated( + replacement: 'destroy_all', + since: '4.5', + )] + public static function kill_all_sessions(): void { + \core\deprecation::emit_deprecation_if_present([self::class, __FUNCTION__]); + self::destroy_all(); } /** * Terminate give session unconditionally. + * * @param string $sid + * @return void + * @deprecated since Moodle 4.5 See MDL-66161 + * @todo Remove in MDL-81848 */ - public static function kill_session($sid) { - global $DB; + #[\core\attribute\deprecated( + replacement: 'destroy', + since: '4.5', + )] + public static function kill_session($sid): void { + \core\deprecation::emit_deprecation_if_present([self::class, __FUNCTION__]); + self::destroy($sid); + } - self::load_handler(); - - if ($sid === session_id()) { - self::write_close(); - } - - self::$handler->kill_session($sid); - - $DB->delete_records('sessions', array('sid'=>$sid)); + /** + * Kill sessions of users with disabled plugins. + * + * @param string $pluginname + * @return void + * @deprecated since Moodle 4.5 See MDL-66161 + * @todo Remove in MDL-81848 + */ + #[\core\attribute\deprecated( + replacement: 'destroy_by_auth_plugin', + since: '4.5', + )] + public static function kill_sessions_for_auth_plugin(string $pluginname): void { + \core\deprecation::emit_deprecation_if_present([self::class, __FUNCTION__]); + self::destroy_by_auth_plugin($pluginname); } /** * Terminate all sessions of given user unconditionally. + * + * @param int $userid + * @param string $keepsid keep this sid if present + * @deprecated since Moodle 4.5 See MDL-66161 + * @todo Remove in MDL-81848 + */ + #[\core\attribute\deprecated( + replacement: 'destroy_user_sessions', + since: '4.5', + )] + public static function kill_user_sessions($userid, $keepsid = null) { + \core\deprecation::emit_deprecation_if_present([self::class, __FUNCTION__]); + self::destroy_user_sessions($userid, $keepsid); + } + + /** + * Destroy all sessions for a given plugin. + * Typically used when a plugin is disabled or uninstalled, so all sessions (users) for that plugin are logged out. + * + * @param string $pluginname Auth plugin name. + */ + public static function destroy_by_auth_plugin(string $pluginname): void { + self::$handler->destroy_by_auth_plugin($pluginname); + } + + /** + * Destroy all sessions, and delete all the session data. + * + * @return bool + */ + public static function destroy_all(): bool { + self::terminate_current(); + self::load_handler(); + + try { + $result = self::$handler->destroy_all(); + } catch (\moodle_exception $ignored) { + // Do not show any warnings - might be during upgrade/installation. + $result = true; + } + + return $result; + } + + /** + * Destroy a specific session and delete this session record for this session id. + * + * @param string $id + * @return bool + */ + public static function destroy(string $id): bool { + self::load_handler(); + + if ($id === session_id()) { + self::write_close(); + } + + return self::$handler->destroy($id); + } + + /** + * Destroy all sessions of given user unconditionally. * @param int $userid * @param string $keepsid keep this sid if present */ - public static function kill_user_sessions($userid, $keepsid = null) { - global $DB; - - $sessions = $DB->get_records('sessions', array('userid'=>$userid), 'id DESC', 'id, sid'); + public static function destroy_user_sessions($userid, $keepsid = null) { + $sessions = self::get_sessions_by_userid($userid); foreach ($sessions as $session) { if ($keepsid and $keepsid === $session->sid) { continue; } - self::kill_session($session->sid); + self::destroy($session->sid); } } @@ -949,30 +1040,35 @@ class manager { return; } - $count = $DB->count_records('sessions', array('userid' => $userid)); + $sessions = self::get_sessions_by_userid($userid); + + $count = count($sessions); if ($count <= $CFG->limitconcurrentlogins) { return; } $i = 0; - $select = "userid = :userid"; - $params = array('userid' => $userid); if ($sid) { - if ($DB->record_exists('sessions', array('sid' => $sid, 'userid' => $userid))) { - $select .= " AND sid <> :sid"; - $params['sid'] = $sid; - $i = 1; + foreach ($sessions as $key => $session) { + if ($session->sid == $sid && $session->userid == $userid) { + $i = 1; + unset($sessions[$key]); + } } } - $sessions = $DB->get_records_select('sessions', $select, $params, 'timecreated DESC', 'id, sid'); + // Order records by timecreated DESC. + usort($sessions, function($a, $b){ + return $b->timecreated <=> $a->timecreated; + }); + foreach ($sessions as $session) { $i++; if ($i <= $CFG->limitconcurrentlogins) { continue; } - self::kill_session($session->sid); + self::destroy($session->sid); } } @@ -1006,86 +1102,18 @@ class manager { /** * Periodic timed-out session cleanup. + * + * @param int $maxlifetime Sessions that have not updated for the last max_lifetime seconds will be removed. + * @return void */ - public static function gc() { - global $CFG, $DB; + public static function gc(int $maxlifetime = 0): void { + global $CFG; - // This may take a long time... - \core_php_time_limit::raise(); - - $maxlifetime = $CFG->sessiontimeout; - - try { - // Kill all sessions of deleted and suspended users without any hesitation. - $rs = $DB->get_recordset_select('sessions', "userid IN (SELECT id FROM {user} WHERE deleted <> 0 OR suspended <> 0)", array(), 'id DESC', 'id, sid'); - foreach ($rs as $session) { - self::kill_session($session->sid); - } - $rs->close(); - - // Kill sessions of users with disabled plugins. - $authsequence = get_enabled_auth_plugins(); - $authsequence = array_flip($authsequence); - unset($authsequence['nologin']); // No login means user cannot login. - $authsequence = array_flip($authsequence); - - list($notplugins, $params) = $DB->get_in_or_equal($authsequence, SQL_PARAMS_QM, '', false); - $rs = $DB->get_recordset_select('sessions', "userid IN (SELECT id FROM {user} WHERE auth $notplugins)", $params, 'id DESC', 'id, sid'); - foreach ($rs as $session) { - self::kill_session($session->sid); - } - $rs->close(); - - // Now get a list of time-out candidates - real users only. - $sql = "SELECT u.*, s.sid, s.timecreated AS s_timecreated, s.timemodified AS s_timemodified - FROM {user} u - JOIN {sessions} s ON s.userid = u.id - WHERE s.timemodified < :purgebefore AND u.id <> :guestid"; - $params = array('purgebefore' => (time() - $maxlifetime), 'guestid'=>$CFG->siteguest); - - $authplugins = array(); - foreach ($authsequence as $authname) { - $authplugins[$authname] = get_auth_plugin($authname); - } - $rs = $DB->get_recordset_sql($sql, $params); - foreach ($rs as $user) { - foreach ($authplugins as $authplugin) { - /** @var \auth_plugin_base $authplugin*/ - if ($authplugin->ignore_timeout_hook($user, $user->sid, $user->s_timecreated, $user->s_timemodified)) { - continue 2; - } - } - self::kill_session($user->sid); - } - $rs->close(); - - // Delete expired sessions for guest user account, give them larger timeout, there is no security risk here. - $params = array('purgebefore' => (time() - ($maxlifetime * 5)), 'guestid'=>$CFG->siteguest); - $rs = $DB->get_recordset_select('sessions', 'userid = :guestid AND timemodified < :purgebefore', $params, 'id DESC', 'id, sid'); - foreach ($rs as $session) { - self::kill_session($session->sid); - } - $rs->close(); - - // Delete expired sessions for userid = 0 (not logged in), better kill them asap to release memory. - $params = array('purgebefore' => (time() - $maxlifetime)); - $rs = $DB->get_recordset_select('sessions', 'userid = 0 AND timemodified < :purgebefore', $params, 'id DESC', 'id, sid'); - foreach ($rs as $session) { - self::kill_session($session->sid); - } - $rs->close(); - - // Cleanup letfovers from the first browser access because it may set multiple cookies and then use only one. - $params = array('purgebefore' => (time() - 60*3)); - $rs = $DB->get_recordset_select('sessions', 'userid = 0 AND timemodified = timecreated AND timemodified < :purgebefore', $params, 'id ASC', 'id, sid'); - foreach ($rs as $session) { - self::kill_session($session->sid); - } - $rs->close(); - - } catch (\Exception $ex) { - debugging('Error gc-ing sessions: '.$ex->getMessage(), DEBUG_NORMAL, $ex->getTrace()); + // If max lifetime is not provided, use the default session timeout. + if ($maxlifetime == 0) { + $maxlifetime = $CFG->sessiontimeout; } + self::$handler->gc($maxlifetime); } /** @@ -1214,7 +1242,7 @@ class manager { $state = [ 'token' => random_string(32), - 'created' => time() // Server time - not user time. + 'created' => di::get(clock::class)->time(), // Server time - not user time. ]; if (!isset($SESSION->logintoken)) { @@ -1254,7 +1282,7 @@ class manager { } // Check token lifespan. - if ($state['created'] < (time() - $CFG->sessiontimeout)) { + if ($state['created'] < (di::get(clock::class)->time() - $CFG->sessiontimeout)) { $state = self::create_login_token(); } diff --git a/lib/classes/session/memcached.php b/lib/classes/session/memcached.php index 7b76be86e5c..91fb2d5dd2f 100644 --- a/lib/classes/session/memcached.php +++ b/lib/classes/session/memcached.php @@ -14,18 +14,8 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -/** - * Memcached based session handler. - * - * @package core - * @copyright 2013 Petr Skoda {@link http://skodak.org} - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - namespace core\session; -defined('MOODLE_INTERNAL') || die(); - /** * Memcached based session handler. * @@ -96,10 +86,7 @@ class memcached extends handler { } } - /** - * Start the session. - * @return bool success - */ + #[\Override] public function start() { ini_set('memcached.sess_locking', $this->requires_write_lock() ? '1' : '0'); @@ -132,9 +119,7 @@ class memcached extends handler { return $result; } - /** - * Init session handler. - */ + #[\Override] public function init() { if (!extension_loaded('memcached')) { throw new exception('sessionhandlerproblem', 'error', '', null, 'memcached extension is not loaded'); @@ -178,14 +163,7 @@ class memcached extends handler { } - /** - * Check the backend contains data for this session id. - * - * Note: this is intended to be called from manager::session_exists() only. - * - * @param string $sid - * @return bool true if session found. - */ + #[\Override] public function session_exists($sid) { if (!$this->servers) { return false; @@ -209,14 +187,11 @@ class memcached extends handler { return false; } - /** - * Kill all active sessions, the core sessions table is - * purged afterwards. - */ - public function kill_all_sessions() { + #[\Override] + public function destroy_all(): bool { global $DB; if (!$this->servers) { - return; + return false; } // Go through the list of all servers because @@ -245,15 +220,14 @@ class memcached extends handler { foreach ($memcacheds as $memcached) { $memcached->quit(); } + + return true; } - /** - * Kill one session, the session record is removed afterwards. - * @param string $sid - */ - public function kill_session($sid) { + #[\Override] + public function destroy(string $id): bool { if (!$this->servers) { - return; + return false; } // Go through the list of all servers because @@ -264,9 +238,11 @@ class memcached extends handler { list($host, $port) = $server; $memcached = new \Memcached(); $memcached->addServer($host, $port); - $memcached->delete($this->prefix . $sid); + $memcached->delete($this->prefix . $id); $memcached->quit(); } + + return true; } /** diff --git a/lib/classes/session/redis.php b/lib/classes/session/redis.php index bd69c3f3ee4..579ddd52873 100644 --- a/lib/classes/session/redis.php +++ b/lib/classes/session/redis.php @@ -14,29 +14,21 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -/** - * Redis based session handler. - * - * @package core - * @copyright 2015 Russell Smith - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - namespace core\session; -use RedisException; +use coding_exception; +use core\di; +use core\clock; +use RedisCluster; use RedisClusterException; +use RedisException; use SessionHandlerInterface; /** * Redis based session handler. * - * The default Redis session handler does not handle locking in 2.2.7, so we have written a php session handler - * that uses locking. The places where locking is used was modeled from the memcached code that is used in Moodle - * https://github.com/php-memcached-dev/php-memcached/blob/master/php_memcached_session.c - * * @package core - * @copyright 2016 Russell Smith + * @copyright Russell Smith * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class redis extends handler implements SessionHandlerInterface { @@ -53,14 +45,11 @@ class redis extends handler implements SessionHandlerInterface { */ const COMPRESSION_ZSTD = 'zstd'; - /** - * Minimum version of the Redis extension required. - */ - public const REDIS_EXTENSION_MIN_VERSION = '2.2.4'; - /** - * Minimum version of the Redis extension required. - */ - private const REDIS_SERVER_MIN_VERSION = '2.6.12'; + /** @var string Minimum server version */ + const REDIS_MIN_SERVER_VERSION = "5.0.0"; + + /** @var string Minimum extension version */ + const REDIS_MIN_EXTENSION_VERSION = "5.1.0"; /** @var array $host save_path string */ protected array $host = []; @@ -74,6 +63,13 @@ class redis extends handler implements SessionHandlerInterface { protected $database = 0; /** @var array $servers list of servers parsed from save_path */ protected $prefix = ''; + + /** @var string $sessionkeyprefix the prefix for the session key */ + protected string $sessionkeyprefix = 'session_'; + + /** @var string $userkeyprefix the prefix for the user key */ + protected string $userkeyprefix = 'user_'; + /** @var int $acquiretimeout how long to wait for session lock in seconds */ protected $acquiretimeout = 120; /** @var int $acquirewarn how long before warning when waiting for a lock in seconds */ @@ -87,14 +83,18 @@ class redis extends handler implements SessionHandlerInterface { /** @var string $lasthash hash of the session data content */ protected $lasthash = null; - /** - * @var int $lockexpire how long to wait in seconds before expiring the lock automatically - * so that other requests may continue execution, ignored if PECL redis is below version 2.2.0. - */ - protected $lockexpire; + /** @var int $gcbatchsize The number of redis keys that will be processed each time the garbage collector is executed. */ + protected int $gcbatchsize = 100; - /** @var Redis|RedisCluster Connection */ - protected $connection = null; + /** + * How long to wait in seconds before expiring the lock automatically so that other requests may continue execution. + * + * @var int $lockexpire + */ + protected int $lockexpire; + + /** @var \Redis|\RedisCluster|null Connection */ + protected \Redis|\RedisCluster|null $connection = null; /** @var array $locks List of currently held locks by this page. */ protected $locks = array(); @@ -108,6 +108,12 @@ class redis extends handler implements SessionHandlerInterface { /** @var int Maximum number of retries for cache store operations. */ const MAX_RETRIES = 5; + /** @var int $firstaccesstimeout The initial timeout (seconds) for the first browser access without login. */ + protected int $firstaccesstimeout = 180; + + /** @var clock A clock instance */ + protected clock $clock; + /** * Create new instance of handler. */ @@ -157,12 +163,6 @@ class redis extends handler implements SessionHandlerInterface { $this->serializer = \Redis::SERIALIZER_IGBINARY; // Set igbinary serializer if phpredis supports it. } - // The following configures the session lifetime in redis to allow some - // wriggle room in the user noticing they've been booted off and - // letting them log back in before they lose their session entirely. - $updatefreq = empty($CFG->session_update_timemodified_frequency) ? 20 : $CFG->session_update_timemodified_frequency; - $this->timeout = $CFG->sessiontimeout + $updatefreq + MINSECS; - // This sets the Redis session lock expiry time to whatever is lower, either // the PHP execution time `max_execution_time`, if the value was defined in // the `php.ini` or the globally configured `sessiontimeout`. Setting it to @@ -187,37 +187,33 @@ class redis extends handler implements SessionHandlerInterface { if (isset($CFG->session_redis_compressor)) { $this->compressor = $CFG->session_redis_compressor; } + + $this->clock = di::get(clock::class); } - /** - * Start the session. - * - * @return bool success - */ - public function start() { - $result = parent::start(); - - return $result; - } - - /** - * Init session handler. - */ - public function init() { + #[\Override] + public function init(): bool { if (!extension_loaded('redis')) { throw new exception('sessionhandlerproblem', 'error', '', null, 'redis extension is not loaded'); } if (empty($this->host)) { - throw new exception('sessionhandlerproblem', 'error', '', null, - '$CFG->session_redis_host must be specified in config.php'); + throw new exception( + 'sessionhandlerproblem', + 'error', + '', + null, + '$CFG->session_redis_host must be specified in config.php', + ); } - // The session handler requires a version of PHP Redis extension with support for SET command options (at least 2.2.4). $version = phpversion('Redis'); - if (!$version || version_compare($version, self::REDIS_EXTENSION_MIN_VERSION) <= 0) { - throw new exception('sessionhandlerproblem', 'error', '', null, - 'redis extension version must be at least ' . self::REDIS_EXTENSION_MIN_VERSION); + if (!$version || version_compare($version, self::REDIS_MIN_EXTENSION_VERSION) <= 0) { + throw new exception( + errorcode: 'sessionhandlerproblem', + module: 'error', + debuginfo: sprintf('redis extension version must be at least %s', self::REDIS_MIN_EXTENSION_VERSION), + ); } $result = session_set_save_handler($this); @@ -275,12 +271,26 @@ class redis extends handler implements SessionHandlerInterface { try { // Create a $redis object of a RedisCluster or Redis class. if ($this->clustermode) { - $this->connection = new \RedisCluster(null, $trimmedservers, 1, 1, true, - $this->auth, !empty($opts) ? $opts : null); + $this->connection = new \RedisCluster( + name: null, + seeds: $trimmedservers, + timeout: 1, + readTimeout: 1, + persistent: true, + auth: $this->auth, + context: !empty($opts) ? $opts : null, + ); } else { $delay = rand(100, 500); $this->connection = new \Redis(); - $this->connection->connect($server, $port, 1, null, $delay, 1, $opts); + $this->connection->connect( + host: $server, + port: $port, + timeout: 1, + retry_interval: $delay, + read_timeout: 1, + context: $opts, + ); if ($this->auth !== '' && !$this->connection->auth($this->auth)) { throw new $exceptionclass('Unable to authenticate.'); } @@ -295,12 +305,25 @@ class redis extends handler implements SessionHandlerInterface { throw new $exceptionclass('Unable to set the Redis Prefix option.'); } } - if ($this->sslopts && !$this->connection->ping('Ping')) { - // In case of a TLS connection, - // if phpredis client does not communicate immediately with the server the connection hangs. - // See https://github.com/phpredis/phpredis/issues/2332. - throw new $exceptionclass("Ping failed"); + $info = $this->connection->info('server'); + if (!$info) { + throw new $exceptionclass("Failed to fetch server information"); } + + // Check the server version. + // Note: In case of a TLS connection, + // if phpredis client does not communicate immediately with the server the connection hangs. + // See https://github.com/phpredis/phpredis/issues/2332. + // This version check satisfies that requirement. + $version = $info['redis_version']; + if (!$version || version_compare($version, static::REDIS_MIN_SERVER_VERSION) <= 0) { + throw new $exceptionclass(sprintf( + "Version %s is not supported. The minimum version required is %s.", + $version, + static::REDIS_MIN_SERVER_VERSION, + )); + } + if ($this->database !== 0) { if (!$this->connection->select($this->database)) { throw new $exceptionclass('Unable to select the Redis database ' . $this->database . '.'); @@ -309,9 +332,9 @@ class redis extends handler implements SessionHandlerInterface { // The session handler requires a version of Redis server with support for SET command options (at least 2.6.12). $serverversion = $this->connection->info('server')['redis_version']; - if (version_compare($serverversion, self::REDIS_SERVER_MIN_VERSION) <= 0) { + if (version_compare($serverversion, self::REDIS_MIN_SERVER_VERSION) <= 0) { throw new exception('sessionhandlerproblem', 'error', '', null, - 'redis server version must be at least ' . self::REDIS_SERVER_MIN_VERSION); + 'redis server version must be at least ' . self::REDIS_MIN_SERVER_VERSION); } return true; } catch (RedisException | RedisClusterException $e) { @@ -334,6 +357,7 @@ class redis extends handler implements SessionHandlerInterface { if (!$result) { throw new exception('redissessionhandlerproblem', 'error'); } + return false; } /** @@ -356,7 +380,7 @@ class redis extends handler implements SessionHandlerInterface { $this->lasthash = null; try { foreach ($this->locks as $id => $expirytime) { - if ($expirytime > $this->time()) { + if ($expirytime > $this->clock->time()) { $this->unlock_session($id); } unset($this->locks[$id]); @@ -373,29 +397,45 @@ class redis extends handler implements SessionHandlerInterface { * Read the session data from storage * * @param string $id The session id to read from storage. - * @return string The session data for PHP to process. + * @return string|false The session data for PHP to process or false. * * @throws RedisException when we are unable to talk to the Redis server. */ public function read(string $id): string|false { try { if ($this->requires_write_lock()) { - $this->lock_session($id); + $this->lock_session($this->sessionkeyprefix . $id); } - $sessiondata = $this->uncompress($this->connection->get($id)); + + $keys = $this->connection->hmget($this->sessionkeyprefix . $id, ['userid', 'sessdata']); + $userid = $keys['userid']; + $sessiondata = $this->uncompress($keys['sessdata']); if ($sessiondata === false) { if ($this->requires_write_lock()) { - $this->unlock_session($id); + $this->unlock_session($this->sessionkeyprefix . $id); } $this->lasthash = sha1(''); return ''; } - $this->connection->expire($id, $this->timeout); + + // Do not update expiry if non-login user (0). This would affect the first access timeout. + if ($userid != 0) { + $maxlifetime = $this->get_maxlifetime($userid); + $this->connection->expire($this->sessionkeyprefix . $id, $maxlifetime); + $this->connection->expire($this->userkeyprefix . $userid, $maxlifetime); + } } catch (RedisException | RedisClusterException $e) { error_log('Failed talking to redis: '.$e->getMessage()); throw $e; } + + // Update last hash. + if ($sessiondata === null) { + // As of PHP 8.1 we can't pass null to base64_encode. + $sessiondata = ''; + } + $this->lasthash = sha1(base64_encode($sessiondata)); return $sessiondata; } @@ -406,7 +446,7 @@ class redis extends handler implements SessionHandlerInterface { * @param mixed $value * @return string */ - private function compress($value) { + private function compress($value): string { switch ($this->compressor) { case self::COMPRESSION_NONE: return $value; @@ -455,7 +495,6 @@ class redis extends handler implements SessionHandlerInterface { * @return bool true on write success, false on failure */ public function write(string $id, string $data): bool { - $hash = sha1(base64_encode($data)); // If the content has not changed don't bother writing. @@ -475,8 +514,16 @@ class redis extends handler implements SessionHandlerInterface { // address that in the future. try { $data = $this->compress($data); + $this->connection->hset($this->sessionkeyprefix . $id, 'sessdata', $data); + $keys = $this->connection->hmget($this->sessionkeyprefix . $id, ['userid', 'timecreated', 'timemodified']); + $userid = $keys['userid']; - $this->connection->setex($id, $this->timeout, $data); + // Don't update expiry if still first access. + if ($keys['timecreated'] != $keys['timemodified']) { + $maxlifetime = $this->get_maxlifetime($userid); + $this->connection->expire($this->sessionkeyprefix . $id, $maxlifetime); + $this->connection->expire($this->userkeyprefix . $userid, $maxlifetime); + } } catch (RedisException | RedisClusterException $e) { error_log('Failed talking to redis: '.$e->getMessage()); return false; @@ -484,16 +531,124 @@ class redis extends handler implements SessionHandlerInterface { return true; } - /** - * Handle destroying a session. - * - * @param string $id the session id to destroy. - * @return bool true if the session was deleted, false otherwise. - */ + #[\Override] + public function get_session_by_sid(string $sid): \stdClass { + $keys = ["id", "state", "sid", "userid", "sessdata", "timecreated", "timemodified", "firstip", "lastip"]; + $sessiondata = $this->connection->hmget($this->sessionkeyprefix . $sid, $keys); + + return (object)$sessiondata; + } + + #[\Override] + public function add_session(int $userid): \stdClass { + $timestamp = $this->clock->time(); + $sid = session_id(); + $maxlifetime = $this->get_maxlifetime($userid, true); + $sessiondata = [ + 'id' => $sid, + 'state' => '0', + 'sid' => $sid, + 'userid' => $userid, + 'sessdata' => null, + 'timecreated' => $timestamp, + 'timemodified' => $timestamp, + 'firstip' => getremoteaddr(), + 'lastip' => getremoteaddr(), + ]; + + $userhashkey = $this->userkeyprefix . $userid; + $this->connection->hSet($userhashkey, $sid, $timestamp); + $this->connection->expire($userhashkey, $maxlifetime); + + $sessionhashkey = $this->sessionkeyprefix . $sid; + $this->connection->hmSet($sessionhashkey, $sessiondata); + $this->connection->expire($sessionhashkey, $maxlifetime); + + return (object)$sessiondata; + } + + #[\Override] + public function get_sessions_by_userid(int $userid): array { + $this->init_redis_if_required(); + + $userhashkey = $this->userkeyprefix . $userid; + $sessions = $this->connection->hGetAll($userhashkey); + $records = []; + foreach (array_keys($sessions) as $session) { + $item = $this->connection->hGetAll($this->sessionkeyprefix . $session); + if (!empty($item)) { + $records[] = (object) $item; + } + } + return $records; + } + + #[\Override] + public function update_session(\stdClass $record): bool { + if (!isset($record->sid) && isset($record->id)) { + $record->sid = $record->id; + } + + // If record does not have userid set, we need to get it from the session. + if (!isset($record->userid)) { + $session = $this->get_session_by_sid($record->sid); + $record->userid = $session->userid; + } + + $sessionhashkey = $this->sessionkeyprefix . $record->sid; + $userhashkey = $this->userkeyprefix . $record->userid; + + $recordata = (array) $record; + unset($recordata['sid']); + $this->connection->hmSet($sessionhashkey, $recordata); + + // Update the expiry time. + $maxlifetime = $this->get_maxlifetime($record->userid); + $this->connection->expire($sessionhashkey, $maxlifetime); + $this->connection->expire($userhashkey, $maxlifetime); + + return true; + } + + + #[\Override] + public function get_all_sessions(): \Iterator { + $sessions = []; + $iterator = null; + while (false !== ($keys = $this->connection->scan($iterator, '*' . $this->sessionkeyprefix . '*'))) { + foreach ($keys as $key) { + $sessions[] = $key; + } + } + return new \ArrayIterator($sessions); + } + + #[\Override] + public function destroy_all(): bool { + $this->init_redis_if_required(); + + $sessions = $this->get_all_sessions(); + foreach ($sessions as $session) { + // Remove the prefixes from the session id, as destroy expects the raw session id. + if (str_starts_with($session, $this->prefix . $this->sessionkeyprefix)) { + $session = substr($session, strlen($this->prefix . $this->sessionkeyprefix)); + } + + $this->destroy($session); + } + return true; + } + + #[\Override] public function destroy(string $id): bool { + $this->init_redis_if_required(); $this->lasthash = null; try { - $this->connection->del($id); + $sessionhashkey = $this->sessionkeyprefix . $id; + $userid = $this->connection->hget($sessionhashkey, "userid"); + $userhashkey = $this->userkeyprefix . $userid; + $this->connection->hDel($userhashkey, $id); + $this->connection->unlink($sessionhashkey); $this->unlock_session($id); } catch (RedisException | RedisClusterException $e) { error_log('Failed talking to redis: '.$e->getMessage()); @@ -503,15 +658,53 @@ class redis extends handler implements SessionHandlerInterface { return true; } + // phpcs:disable moodle.NamingConventions.ValidVariableName.VariableNameUnderscore + #[\Override] + public function gc(int $max_lifetime = 0): int|false { + return 0; + } + // phpcs:enable + /** - * Garbage collect sessions. We don't we any as Redis does it for us. + * Get session maximum lifetime in seconds. * - * @param integer $max_lifetime All sessions older than this should be removed. - * @return bool true, as Redis handles expiry for us. + * @param int|null $userid The user id to calculate the max lifetime for. + * @param bool $firstbrowseraccess This indicates that this is calculating the expiry when the key is first added. + * The first access made by the browser has a shorter timeout to reduce abandoned sessions. + * @return float|int */ - // phpcs:ignore moodle.NamingConventions.ValidVariableName.VariableNameUnderscore - public function gc(int $max_lifetime): int|false { - return false; + private function get_maxlifetime(?int $userid = null, bool $firstbrowseraccess = false): float|int { + global $CFG; + + // Guest user. + if ($userid == $CFG->siteguest) { + return $CFG->sessiontimeout * 5; + } + + // All other users. + if ($userid == 0 && $firstbrowseraccess) { + $maxlifetime = $this->firstaccesstimeout; + } else { + // As per MDL-56823 - The following configures the session lifetime in redis to allow some + // wriggle room in the user noticing they've been booted off and + // letting them log back in before they lose their session entirely. + $updatefreq = empty($CFG->session_update_timemodified_frequency) ? 20 : $CFG->session_update_timemodified_frequency; + $maxlifetime = (int) $CFG->sessiontimeout + $updatefreq + MINSECS; + } + + return $maxlifetime; + } + + /** + * Connection will be null if these methods are called from cli or where NO_MOODLE_COOKIES is used. + * We need to check for this and initialize the connection if required. + * + * @return void + */ + private function init_redis_if_required(): void { + if (is_null($this->connection)) { + $this->init(); + } } /** @@ -521,7 +714,7 @@ class redis extends handler implements SessionHandlerInterface { */ protected function unlock_session($id) { if (isset($this->locks[$id])) { - $this->connection->del($id.".lock"); + $this->connection->unlink("{$id}.lock"); unset($this->locks[$id]); } } @@ -534,25 +727,30 @@ class redis extends handler implements SessionHandlerInterface { * @throws exception When we are unable to obtain a session lock. */ protected function lock_session($id) { - $lockkey = $id.".lock"; + $lockkey = "{$id}.lock"; - $haslock = isset($this->locks[$id]) && $this->time() < $this->locks[$id]; - $startlocktime = $this->time(); + $haslock = isset($this->locks[$id]) && $this->clock->time() < $this->locks[$id]; + if ($haslock) { + return true; + } - /* To be able to ensure sessions don't write out of order we must obtain an exclusive lock - * on the session for the entire time it is open. If another AJAX call, or page is using - * the session then we just wait until it finishes before we can open the session. - */ + $startlocktime = $this->clock->time(); + + // To be able to ensure sessions don't write out of order we must obtain an exclusive lock + // on the session for the entire time it is open. If another AJAX call, or page is using + // the session then we just wait until it finishes before we can open the session. // Store the current host, process id and the request URI so it's easy to track who has the lock. $hostname = gethostname(); if ($hostname === false) { $hostname = 'UNKNOWN HOST'; } + $pid = getmypid(); if ($pid === false) { $pid = 'UNKNOWN'; } + $uri = isset($_SERVER['REQUEST_URI']) ? $_SERVER['REQUEST_URI'] : 'unknown uri'; $whoami = "[pid {$pid}] {$hostname}:$uri"; @@ -563,38 +761,43 @@ class redis extends handler implements SessionHandlerInterface { $haslock = $this->connection->set($lockkey, $whoami, ['nx', 'ex' => $this->lockexpire]); if ($haslock) { - $this->locks[$id] = $this->time() + $this->lockexpire; + $this->locks[$id] = $this->clock->time() + $this->lockexpire; return true; } - if (!empty($this->acquirewarn) && !$haswarned && $this->time() > $startlocktime + $this->acquirewarn) { + if (!empty($this->acquirewarn) && !$haswarned && $this->clock->time() > $startlocktime + $this->acquirewarn) { // This is a warning to better inform users. $whohaslock = $this->connection->get($lockkey); // phpcs:ignore - error_log("Warning: Cannot obtain session lock for sid: $id within $this->acquirewarn seconds but will keep trying. " . - "It is likely another page ($whohaslock) has a long session lock, or the session lock was never released."); + error_log( + "Warning: Cannot obtain session lock for sid: $id within $this->acquirewarn seconds but will keep trying. " . + "It is likely another page ($whohaslock) has a long session lock, or the session lock was never released.", + ); $haswarned = true; } - if ($this->time() > $startlocktime + $this->acquiretimeout) { + if ($this->clock->time() > $startlocktime + $this->acquiretimeout) { // This is a fatal error, better inform users. // It should not happen very often - all pages that need long time to execute // should close session immediately after access control checks. $whohaslock = $this->connection->get($lockkey); // phpcs:ignore - error_log("Error: Cannot obtain session lock for sid: $id within $this->acquiretimeout seconds. " . - "It is likely another page ($whohaslock) has a long session lock, or the session lock was never released."); + error_log( + "Error: Cannot obtain session lock for sid: $id within $this->acquiretimeout seconds. " . + "It is likely another page ($whohaslock) has a long session lock, or the session lock was never released.", + ); $acquiretimeout = format_time($this->acquiretimeout); $lockexpire = format_time($this->lockexpire); $a = (object)[ 'id' => substr($id, 0, 10), 'acquiretimeout' => $acquiretimeout, 'whohaslock' => $whohaslock, - 'lockexpire' => $lockexpire]; + 'lockexpire' => $lockexpire, + ]; throw new exception("sessioncannotobtainlock", 'error', '', $a); } - if ($this->time() < $startlocktime + 5) { + if ($this->clock->time() < $startlocktime + 5) { // We want a random delay to stagger the polling load. Ideally // this delay should be a fraction of the average response // time. If it is too small we will poll too much and if it is @@ -610,63 +813,20 @@ class redis extends handler implements SessionHandlerInterface { usleep($delay * 1000); } + throw new coding_exception('Unable to lock session'); } - /** - * Return the current time. - * - * @return int the current time as a unixtimestamp. - */ - protected function time() { - return time(); - } - - /** - * Check the backend contains data for this session id. - * - * Note: this is intended to be called from manager::session_exists() only. - * - * @param string $sid - * @return bool true if session found. - */ + #[\Override] public function session_exists($sid) { if (!$this->connection) { return false; } try { - return !empty($this->connection->exists($sid)); + $sessionhashkey = $this->sessionkeyprefix . $sid; + return !empty($this->connection->exists($sessionhashkey)); } catch (RedisException | RedisClusterException $e) { return false; } } - - /** - * Kill all active sessions, the core sessions table is purged afterwards. - */ - public function kill_all_sessions() { - global $DB; - if (!$this->connection) { - return; - } - - $rs = $DB->get_recordset('sessions', array(), 'id DESC', 'id, sid'); - foreach ($rs as $record) { - $this->destroy($record->sid); - } - $rs->close(); - } - - /** - * Kill one session, the session record is removed afterwards. - * - * @param string $sid - */ - public function kill_session($sid) { - if (!$this->connection) { - return; - } - - $this->destroy($sid); - } } diff --git a/lib/moodlelib.php b/lib/moodlelib.php index c7259cc7544..8d6bf0992d9 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -2689,7 +2689,8 @@ function require_logout() { 'other' => array('sessionid' => $sid), ) ); - if ($session = $DB->get_record('sessions', array('sid'=>$sid))) { + $session = \core\session\manager::get_session_by_sid($sid); + if (isset($session->id)) { $event->add_record_snapshot('sessions', $session); } @@ -3675,7 +3676,7 @@ function delete_user(stdClass $user) { } // Force logout - may fail if file based sessions used, sorry. - \core\session\manager::kill_user_sessions($user->id); + \core\session\manager::destroy_user_sessions($user->id); // Generate username from email address, or a fake email. $delemail = !empty($user->email) ? $user->email : $user->username . '.' . $user->id . '@unknownemail.invalid'; diff --git a/lib/tests/classes/session/mock_handler.php b/lib/tests/classes/session/mock_handler.php new file mode 100644 index 00000000000..b484b42b01a --- /dev/null +++ b/lib/tests/classes/session/mock_handler.php @@ -0,0 +1,86 @@ +. + +namespace core\tests\session; + +use core\clock; +use core\di; +use core\session\database; + +/** + * Mock handler methods class. + * + * @package core + * @author Darren Cocco + * @author Trisha Milan + * @copyright 2022 Monash University (http://www.monash.edu) + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class mock_handler extends database { + #[\Override] + public function init(): bool { + // Nothing special to do in the mock. + return true; + } + + #[\Override] + public function session_exists($sid): bool { + global $DB; + + return $DB->record_exists('sessions', ['sid' => $sid]); + } + + /** + * Insert a new session record to be used in unit tests. + * + * @param \stdClass $record + * @return int Inserted record id. + */ + public function add_test_session(\stdClass $record): int { + global $DB, $USER; + + $data = new \stdClass(); + $data->state = $record->state ?? 0; + $data->sid = $record->sid ?? session_id(); + $data->sessdata = $record->sessdata ?? null; + $data->userid = $record->userid ?? $USER->id; + $data->timecreated = $record->timecreated ?? di::get(clock::class)->time(); + $data->timemodified = $record->timemodified ?? di::get(clock::class)->time(); + $data->firstip = $record->firstip ?? getremoteaddr(); + $data->lastip = $record->lastip ?? getremoteaddr(); + + return $DB->insert_record('sessions', $data); + } + + #[\Override] + public function get_all_sessions(): \Iterator { + global $DB; + + $records = $DB->get_records('sessions'); + return new \ArrayIterator($records); + } + + /** + * Returns the number of all sessions stored. + * + * @return int + */ + public function count_sessions(): int { + global $DB; + + return $DB->count_records('sessions'); + } +} diff --git a/lib/tests/session_manager_test.php b/lib/tests/session/manager_test.php similarity index 73% rename from lib/tests/session_manager_test.php rename to lib/tests/session/manager_test.php index 5da9d95749a..0c685c59863 100644 --- a/lib/tests/session_manager_test.php +++ b/lib/tests/session/manager_test.php @@ -14,7 +14,9 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -namespace core; +namespace core\session; + +use core\tests\session\mock_handler; /** * Unit tests for session manager class. @@ -23,8 +25,18 @@ namespace core; * @category test * @copyright 2013 Petr Skoda {@link http://skodak.org} * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \core\session\manager */ -class session_manager_test extends \advanced_testcase { +final class manager_test extends \advanced_testcase { + + /** @var mock_handler $mockhandler Dedicated testing handler. */ + protected mock_handler $mockhandler; + + protected function setUp(): void { + parent::setUp(); + $this->mockhandler = new mock_handler(); + } + public function test_start(): void { $this->resetAfterTest(); // Session must be started only once... @@ -185,23 +197,23 @@ class session_manager_test extends \advanced_testcase { $record->sid = $sid; $record->timecreated = time(); $record->timemodified = $record->timecreated; - $record->id = $DB->insert_record('sessions', $record); + $record->id = $this->mockhandler->add_test_session($record); $this->assertTrue(\core\session\manager::session_exists($sid)); $record->timecreated = time() - $CFG->sessiontimeout - 100; $record->timemodified = $record->timecreated + 10; - $DB->update_record('sessions', $record); + \core\session\manager::update_session($record); $this->assertTrue(\core\session\manager::session_exists($sid)); $record->userid = $guest->id; - $DB->update_record('sessions', $record); + \core\session\manager::update_session($record); $this->assertTrue(\core\session\manager::session_exists($sid)); $record->userid = $user->id; - $DB->update_record('sessions', $record); + \core\session\manager::update_session($record); $this->assertFalse(\core\session\manager::session_exists($sid)); @@ -211,7 +223,6 @@ class session_manager_test extends \advanced_testcase { } public function test_touch_session(): void { - global $DB; $this->resetAfterTest(); $sid = md5('hokus'); @@ -223,17 +234,23 @@ class session_manager_test extends \advanced_testcase { $record->timecreated = time() - 60*60; $record->timemodified = time() - 30; $record->firstip = $record->lastip = '10.0.0.1'; - $record->id = $DB->insert_record('sessions', $record); + $record->id = $this->mockhandler->add_test_session($record); $now = time(); \core\session\manager::touch_session($sid); - $updated = $DB->get_field('sessions', 'timemodified', array('id'=>$record->id)); + $session = \core\session\manager::get_session_by_sid($sid); - $this->assertGreaterThanOrEqual($now, $updated); - $this->assertLessThanOrEqual(time(), $updated); + $this->assertGreaterThanOrEqual($now, $session->timemodified); + $this->assertLessThanOrEqual(time(), $session->timemodified); } - public function test_kill_session(): void { + /** + * Test destroy method. + * + * @return void + * @throws \dml_exception + */ + public function test_destroy(): void { global $DB, $USER; $this->resetAfterTest(); @@ -249,23 +266,23 @@ class session_manager_test extends \advanced_testcase { $record->timecreated = time() - 60*60; $record->timemodified = time() - 30; $record->firstip = $record->lastip = '10.0.0.1'; - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); $record->userid = 0; $record->sid = md5('pokus'); - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); - $this->assertEquals(2, $DB->count_records('sessions')); + $this->assertEquals(2, $this->mockhandler->count_sessions()); - \core\session\manager::kill_session($sid); - - $this->assertEquals(1, $DB->count_records('sessions')); - $this->assertFalse($DB->record_exists('sessions', array('sid'=>$sid))); + \core\session\manager::destroy($sid); + $sessions = $this->mockhandler->get_all_sessions(); + $this->assertEquals(1, count($sessions)); + $this->assertFalse($this->contains_session(['sid' => $sid], $sessions)); $this->assertSame($userid, $USER->id); } - public function test_kill_user_sessions(): void { + public function test_destroy_user_sessions(): void { global $DB, $USER; $this->resetAfterTest(); @@ -281,40 +298,44 @@ class session_manager_test extends \advanced_testcase { $record->timecreated = time() - 60*60; $record->timemodified = time() - 30; $record->firstip = $record->lastip = '10.0.0.1'; - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); $record->sid = md5('hokus2'); - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); $record->userid = 0; $record->sid = md5('pokus'); - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); $this->assertEquals(3, $DB->count_records('sessions')); - \core\session\manager::kill_user_sessions($userid); + \core\session\manager::destroy_user_sessions($userid); - $this->assertEquals(1, $DB->count_records('sessions')); - $this->assertFalse($DB->record_exists('sessions', array('userid' => $userid))); + $sessions = $this->mockhandler->get_all_sessions(); + $this->assertEquals(1, count($sessions)); + $this->assertFalse($this->contains_session(['userid' => $userid], $sessions)); $record->userid = $userid; $record->sid = md5('pokus3'); - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); $record->userid = $userid; $record->sid = md5('pokus4'); - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); $record->userid = $userid; $record->sid = md5('pokus5'); - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); - $this->assertEquals(3, $DB->count_records('sessions', array('userid' => $userid))); + $sessions = \core\session\manager::get_sessions_by_userid($userid); + $this->assertCount(3, $sessions); - \core\session\manager::kill_user_sessions($userid, md5('pokus5')); + \core\session\manager::destroy_user_sessions($userid, md5('pokus5')); - $this->assertEquals(1, $DB->count_records('sessions', array('userid' => $userid))); - $this->assertEquals(1, $DB->count_records('sessions', array('userid' => $userid, 'sid' => md5('pokus5')))); + $sessions = \core\session\manager::get_sessions_by_userid($userid); + $session = reset($sessions); + $this->assertCount(1, $sessions); + $this->assertEquals(md5('pokus5'), $session->sid); } public function test_apply_concurrent_login_limit(): void { @@ -334,41 +355,41 @@ class session_manager_test extends \advanced_testcase { $record->sid = md5('hokus1'); $record->timecreated = 20; - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); $record->sid = md5('hokus2'); $record->timecreated = 10; - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); $record->sid = md5('hokus3'); $record->timecreated = 30; - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); $record->userid = $user2->id; $record->sid = md5('pokus1'); $record->timecreated = 20; - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); $record->sid = md5('pokus2'); $record->timecreated = 10; - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); $record->sid = md5('pokus3'); $record->timecreated = 30; - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); $record->timecreated = 10; $record->userid = $guest->id; $record->sid = md5('g1'); - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); $record->sid = md5('g2'); - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); $record->sid = md5('g3'); - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); $record->userid = 0; $record->sid = md5('nl1'); - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); $record->sid = md5('nl2'); - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); $record->sid = md5('nl3'); - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); set_config('limitconcurrentlogins', 0); $this->assertCount(12, $DB->get_records('sessions')); @@ -390,57 +411,103 @@ class session_manager_test extends \advanced_testcase { set_config('limitconcurrentlogins', 2); \core\session\manager::apply_concurrent_login_limit($user1->id); - $this->assertCount(11, $DB->get_records('sessions')); - $this->assertTrue($DB->record_exists('sessions', array('userid' => $user1->id, 'timecreated' => 20))); - $this->assertTrue($DB->record_exists('sessions', array('userid' => $user1->id, 'timecreated' => 30))); - $this->assertFalse($DB->record_exists('sessions', array('userid' => $user1->id, 'timecreated' => 10))); + $sessions = $this->mockhandler->get_all_sessions(); + $this->assertCount(11, $sessions); + $this->assertTrue($this->contains_session(['userid' => $user1->id, 'timecreated' => 20], $sessions)); + $this->assertTrue($this->contains_session(['userid' => $user1->id, 'timecreated' => 30], $sessions)); + $this->assertFalse($this->contains_session(['userid' => $user1->id, 'timecreated' => 10], $sessions)); - $this->assertTrue($DB->record_exists('sessions', array('userid' => $user2->id, 'timecreated' => 20))); - $this->assertTrue($DB->record_exists('sessions', array('userid' => $user2->id, 'timecreated' => 30))); - $this->assertTrue($DB->record_exists('sessions', array('userid' => $user2->id, 'timecreated' => 10))); + $this->assertTrue($this->contains_session(['userid' => $user2->id, 'timecreated' => 20], $sessions)); + $this->assertTrue($this->contains_session(['userid' => $user2->id, 'timecreated' => 30], $sessions)); + $this->assertTrue($this->contains_session(['userid' => $user2->id, 'timecreated' => 10], $sessions)); set_config('limitconcurrentlogins', 2); \core\session\manager::apply_concurrent_login_limit($user2->id, md5('pokus2')); - $this->assertCount(10, $DB->get_records('sessions')); - $this->assertFalse($DB->record_exists('sessions', array('userid' => $user2->id, 'timecreated' => 20))); - $this->assertTrue($DB->record_exists('sessions', array('userid' => $user2->id, 'timecreated' => 30))); - $this->assertTrue($DB->record_exists('sessions', array('userid' => $user2->id, 'timecreated' => 10))); + $sessions = $this->mockhandler->get_all_sessions(); + $this->assertCount(10, $sessions); + $this->assertFalse($this->contains_session(['userid' => $user2->id, 'timecreated' => 20], $sessions)); + $this->assertTrue($this->contains_session(['userid' => $user2->id, 'timecreated' => 30], $sessions)); + $this->assertTrue($this->contains_session(['userid' => $user2->id, 'timecreated' => 10], $sessions)); \core\session\manager::apply_concurrent_login_limit($guest->id); \core\session\manager::apply_concurrent_login_limit(0); - $this->assertCount(10, $DB->get_records('sessions')); + $sessions = $this->mockhandler->get_all_sessions(); + $this->assertCount(10, $sessions); set_config('limitconcurrentlogins', 1); \core\session\manager::apply_concurrent_login_limit($user1->id, md5('grrr')); - $this->assertCount(9, $DB->get_records('sessions')); - $this->assertFalse($DB->record_exists('sessions', array('userid' => $user1->id, 'timecreated' => 20))); - $this->assertTrue($DB->record_exists('sessions', array('userid' => $user1->id, 'timecreated' => 30))); - $this->assertFalse($DB->record_exists('sessions', array('userid' => $user1->id, 'timecreated' => 10))); + $sessions = $this->mockhandler->get_all_sessions(); + $this->assertCount(9, $sessions); + $this->assertFalse($this->contains_session(['userid' => $user1->id, 'timecreated' => 20], $sessions)); + $this->assertTrue($this->contains_session(['userid' => $user1->id, 'timecreated' => 30], $sessions)); + $this->assertFalse($this->contains_session(['userid' => $user1->id, 'timecreated' => 10], $sessions)); \core\session\manager::apply_concurrent_login_limit($user1->id); - $this->assertCount(9, $DB->get_records('sessions')); - $this->assertFalse($DB->record_exists('sessions', array('userid' => $user1->id, 'timecreated' => 20))); - $this->assertTrue($DB->record_exists('sessions', array('userid' => $user1->id, 'timecreated' => 30))); - $this->assertFalse($DB->record_exists('sessions', array('userid' => $user1->id, 'timecreated' => 10))); + $sessions = $this->mockhandler->get_all_sessions(); + $this->assertCount(9, $sessions); + $this->assertFalse($this->contains_session(['userid' => $user1->id, 'timecreated' => 20], $sessions)); + $this->assertTrue($this->contains_session(['userid' => $user1->id, 'timecreated' => 30], $sessions)); + $this->assertFalse($this->contains_session(['userid' => $user1->id, 'timecreated' => 10], $sessions)); \core\session\manager::apply_concurrent_login_limit($user2->id, md5('pokus2')); - $this->assertCount(8, $DB->get_records('sessions')); - $this->assertFalse($DB->record_exists('sessions', array('userid' => $user2->id, 'timecreated' => 20))); - $this->assertFalse($DB->record_exists('sessions', array('userid' => $user2->id, 'timecreated' => 30))); - $this->assertTrue($DB->record_exists('sessions', array('userid' => $user2->id, 'timecreated' => 10))); + $sessions = $this->mockhandler->get_all_sessions(); + $this->assertCount(8, $sessions); + $this->assertFalse($this->contains_session(['userid' => $user2->id, 'timecreated' => 20], $sessions)); + $this->assertFalse($this->contains_session(['userid' => $user2->id, 'timecreated' => 30], $sessions)); + $this->assertTrue($this->contains_session(['userid' => $user2->id, 'timecreated' => 10], $sessions)); \core\session\manager::apply_concurrent_login_limit($user2->id); - $this->assertCount(8, $DB->get_records('sessions')); - $this->assertFalse($DB->record_exists('sessions', array('userid' => $user2->id, 'timecreated' => 20))); - $this->assertFalse($DB->record_exists('sessions', array('userid' => $user2->id, 'timecreated' => 30))); - $this->assertTrue($DB->record_exists('sessions', array('userid' => $user2->id, 'timecreated' => 10))); + $sessions = $this->mockhandler->get_all_sessions(); + $this->assertCount(8, $sessions); + $this->assertFalse($this->contains_session(['userid' => $user2->id, 'timecreated' => 20], $sessions)); + $this->assertFalse($this->contains_session(['userid' => $user2->id, 'timecreated' => 30], $sessions)); + $this->assertTrue($this->contains_session(['userid' => $user2->id, 'timecreated' => 10], $sessions)); \core\session\manager::apply_concurrent_login_limit($guest->id); \core\session\manager::apply_concurrent_login_limit(0); - $this->assertCount(8, $DB->get_records('sessions')); + $sessions = $this->mockhandler->get_all_sessions(); + $this->assertCount(8, $sessions); } - public function test_kill_all_sessions(): void { + /** + * Helper method to check if the sessions array contains a session with the given conditions. + * + * @param array $conditions Conditions to match. + * @param null|\Iterator $sessions Sessions to check. + * @return bool + */ + protected function contains_session(array $conditions, ?\Iterator $sessions = null): bool { + foreach ($sessions as $session) { + if ($this->matches_session($conditions, $session)) { + return true; + } + } + return false; + } + + /** + * Helper method to check if the session matches the given conditions. + * + * @param array $conditions Conditions to match. + * @param \stdClass $session Session to check. + * @return bool + */ + protected function matches_session(array $conditions, \stdClass $session): bool { + foreach ($conditions as $key => $value) { + if ($session->$key != $value) { + return false; + } + } + return true; + } + + /** + * Test destroy_all method. + * + * @return void + * @throws \dml_exception + */ + public function test_destroy_all(): void { global $DB, $USER; $this->resetAfterTest(); @@ -456,25 +523,25 @@ class session_manager_test extends \advanced_testcase { $record->timecreated = time() - 60*60; $record->timemodified = time() - 30; $record->firstip = $record->lastip = '10.0.0.1'; - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); $record->sid = md5('hokus2'); - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); $record->userid = 0; $record->sid = md5('pokus'); - $DB->insert_record('sessions', $record); + $this->mockhandler->add_test_session($record); $this->assertEquals(3, $DB->count_records('sessions')); - \core\session\manager::kill_all_sessions(); + \core\session\manager::destroy_all(); $this->assertEquals(0, $DB->count_records('sessions')); $this->assertSame(0, $USER->id); } public function test_gc(): void { - global $CFG, $DB, $USER; + global $CFG, $USER; $this->resetAfterTest(); $this->setAdminUser(); @@ -483,6 +550,8 @@ class session_manager_test extends \advanced_testcase { $guestid = $USER->id; $this->setUser(0); + // Set sessions timeout to 600 (10 minutes) seconds. + // We will test if sessions not modified for 600 seconds are removed. $CFG->sessiontimeout = 60*10; $record = new \stdClass(); @@ -493,53 +562,53 @@ class session_manager_test extends \advanced_testcase { $record->timecreated = time() - 60*60; $record->timemodified = time() - 30; $record->firstip = $record->lastip = '10.0.0.1'; - $r1 = $DB->insert_record('sessions', $record); + $r1 = $this->mockhandler->add_test_session($record); $record->sid = md5('hokus2'); $record->userid = $adminid; $record->timecreated = time() - 60*60; $record->timemodified = time() - 60*20; - $r2 = $DB->insert_record('sessions', $record); + $r2 = $this->mockhandler->add_test_session($record); $record->sid = md5('hokus3'); $record->userid = $guestid; $record->timecreated = time() - 60*60*60; $record->timemodified = time() - 60*20; - $r3 = $DB->insert_record('sessions', $record); + $r3 = $this->mockhandler->add_test_session($record); $record->sid = md5('hokus4'); $record->userid = $guestid; $record->timecreated = time() - 60*60*60; $record->timemodified = time() - 60*10*5 - 60; - $r4 = $DB->insert_record('sessions', $record); + $r4 = $this->mockhandler->add_test_session($record); $record->sid = md5('hokus5'); $record->userid = 0; $record->timecreated = time() - 60*5; $record->timemodified = time() - 60*5; - $r5 = $DB->insert_record('sessions', $record); + $r5 = $this->mockhandler->add_test_session($record); $record->sid = md5('hokus6'); $record->userid = 0; $record->timecreated = time() - 60*60; $record->timemodified = time() - 60*10 -10; - $r6 = $DB->insert_record('sessions', $record); + $r6 = $this->mockhandler->add_test_session($record); $record->sid = md5('hokus7'); $record->userid = 0; $record->timecreated = time() - 60*60; $record->timemodified = time() - 60*9; - $r7 = $DB->insert_record('sessions', $record); + $r7 = $this->mockhandler->add_test_session($record); - \core\session\manager::gc(); - - $this->assertTrue($DB->record_exists('sessions', array('id'=>$r1))); - $this->assertFalse($DB->record_exists('sessions', array('id'=>$r2))); - $this->assertTrue($DB->record_exists('sessions', array('id'=>$r3))); - $this->assertFalse($DB->record_exists('sessions', array('id'=>$r4))); - $this->assertFalse($DB->record_exists('sessions', array('id'=>$r5))); - $this->assertFalse($DB->record_exists('sessions', array('id'=>$r6))); - $this->assertTrue($DB->record_exists('sessions', array('id'=>$r7))); + \core\session\manager::gc($CFG->sessiontimeout); + $sessions = $this->mockhandler->get_all_sessions(); + $this->assertTrue($this->contains_session(['id' => $r1], $sessions)); + $this->assertFalse($this->contains_session(['id' => $r2], $sessions)); + $this->assertTrue($this->contains_session(['id' => $r3], $sessions)); + $this->assertFalse($this->contains_session(['id' => $r4], $sessions)); + $this->assertFalse($this->contains_session(['id' => $r5], $sessions)); + $this->assertFalse($this->contains_session(['id' => $r6], $sessions)); + $this->assertTrue($this->contains_session(['id' => $r7], $sessions)); } /** @@ -648,7 +717,7 @@ class session_manager_test extends \advanced_testcase { * * @return array */ - public function pages_sessionlocks() { + public function pages_sessionlocks(): array { return [ [ 'url' => '/good.php', @@ -738,7 +807,7 @@ class session_manager_test extends \advanced_testcase { * * @return array */ - public function sessionlock_history() { + public function sessionlock_history(): array { return [ [ 'url' => '/good.php', @@ -849,7 +918,7 @@ class session_manager_test extends \advanced_testcase { * * @return array */ - public function array_session_diff_provider() { + public static function array_session_diff_provider(): array { // Create an instance of this object so the comparison object's identities are the same. // Used in one of the tests below. $compareobjectb = (object) ['array' => 'b']; @@ -920,4 +989,44 @@ class session_manager_test extends \advanced_testcase { $result = $method->invokeArgs(null, [$a, $b]); $this->assertSame($expected, $result); } + + /** + * Test destroy by auth plugin method. + */ + public function test_destroy_by_auth_plugin(): void { + $this->resetAfterTest(); + global $DB; + + // Create test users. + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(['auth' => 'db']); + + // Create sessions for the users. + $user1sid = md5('hokus'); + $record = new \stdClass(); + $record->state = 0; + $record->sid = $user1sid; + $record->sessdata = null; + $record->userid = $user1->id; + $record->timecreated = time() - 60 * 60; + $record->timemodified = time() - 30; + $record->firstip = $record->lastip = '10.0.0.1'; + $this->mockhandler->add_test_session($record); + + $record->sid = md5('pokus'); + $record->userid = $user2->id; + $this->mockhandler->add_test_session($record); + + // Check sessions. + $sessions = $this->mockhandler->get_all_sessions(); + $this->assertEquals(2, count($sessions)); + + // Destroy the session for the user with manual auth plugin. + \core\session\manager::destroy_by_auth_plugin('manual'); + + // Check that the session for the user with manual auth plugin is destroyed. + $sessions = $this->mockhandler->get_all_sessions(); + $this->assertEquals(1, count($sessions)); + $this->assertFalse($this->contains_session(['sid' => $user1sid], $sessions)); + } } diff --git a/lib/tests/session_redis_test.php b/lib/tests/session/redis_test.php similarity index 62% rename from lib/tests/session_redis_test.php rename to lib/tests/session/redis_test.php index d86eefc04cf..a2346ba8586 100644 --- a/lib/tests/session_redis_test.php +++ b/lib/tests/session/redis_test.php @@ -14,8 +14,9 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -namespace core; +namespace core\session; +use core\tests\session\mock_handler; use Redis; use RedisException; @@ -34,21 +35,21 @@ use RedisException; * @copyright 2016 Russell Smith * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later * @runClassInSeparateProcess + * @covers \core\session\redis */ -class session_redis_test extends \advanced_testcase { - - /** @var $keyprefix This key prefix used when testing Redis */ - protected $keyprefix = null; - /** @var $redis The current testing redis connection */ - protected $redis = null; +final class redis_test extends \advanced_testcase { + /** @var string $keyprefix This key prefix used when testing Redis */ + protected string $keyprefix = ''; + /** @var ?Redis $redis The current testing redis connection */ + protected ?Redis $redis = null; /** @var bool $encrypted Is the current testing redis connection encrypted*/ - protected $encrypted = false; + protected bool $encrypted = false; /** @var int $acquiretimeout how long we wait for session lock in seconds when testing Redis */ - protected $acquiretimeout = 1; + protected int $acquiretimeout = 1; /** @var int $lockexpire how long to wait in seconds before expiring the lock when testing Redis */ - protected $lockexpire = 70; - + protected int $lockexpire = 70; + #[\Override] public function setUp(): void { global $CFG; parent::setUp(); @@ -62,8 +63,8 @@ class session_redis_test extends \advanced_testcase { $version = phpversion('Redis'); if (!$version) { $this->markTestSkipped('Redis extension version missing'); - } else if (version_compare($version, \core\session\redis::REDIS_EXTENSION_MIN_VERSION) <= 0) { - $this->markTestSkipped('Redis extension version must be at least ' . \core\session\redis::REDIS_EXTENSION_MIN_VERSION . + } else if (version_compare($version, \core\session\redis::REDIS_MIN_EXTENSION_VERSION) <= 0) { + $this->markTestSkipped('Redis extension version must be at least ' . \core\session\redis::REDIS_MIN_EXTENSION_VERSION . ': now running "' . $version . '"'); } @@ -135,7 +136,7 @@ class session_redis_test extends \advanced_testcase { $this->assertSame('DATA', $sess->read('sess1')); $this->assertTrue($sess->write('sess1', 'DATA-new')); $this->assertTrue($sess->close()); - $this->assertSessionNoLocks(); + $this->assert_session_no_locks(); } public function test_compression_read_and_write_works(): void { @@ -187,16 +188,16 @@ class session_redis_test extends \advanced_testcase { $sessblocked->read('sess1'); $this->fail('Session lock must fail to be obtained.'); } catch (\core\session\exception $e) { - $this->assertStringContainsString("Unable to obtain lock for session id sess1", $e->getMessage()); + $this->assertStringContainsString("Unable to obtain lock for session id session_se", $e->getMessage()); $this->assertStringContainsString('within 1 sec.', $e->getMessage()); $this->assertStringContainsString('session lock timeout (1 min 10 secs) ', $e->getMessage()); - $this->assertStringContainsString('Cannot obtain session lock for sid: sess1', file_get_contents($errorlog)); + $this->assertStringContainsString('Cannot obtain session lock for sid: session_sess1', file_get_contents($errorlog)); } $this->assertTrue($sessblocked->close()); $this->assertTrue($sess->write('sess1', 'DATA-new')); $this->assertTrue($sess->close()); - $this->assertSessionNoLocks(); + $this->assert_session_no_locks(); } public function test_session_is_destroyed_when_it_does_not_exist(): void { @@ -205,7 +206,7 @@ class session_redis_test extends \advanced_testcase { $sess->set_requires_write_lock(true); $this->assertTrue($sess->open('Not used', 'Not used')); $this->assertTrue($sess->destroy('sess-destroy')); - $this->assertSessionNoLocks(); + $this->assert_session_no_locks(); } public function test_session_is_destroyed_when_we_have_it_open(): void { @@ -216,7 +217,7 @@ class session_redis_test extends \advanced_testcase { $this->assertSame('', $sess->read('sess-destroy')); $this->assertTrue($sess->destroy('sess-destroy')); $this->assertTrue($sess->close()); - $this->assertSessionNoLocks(); + $this->assert_session_no_locks(); } public function test_multiple_sessions_do_not_interfere_with_each_other(): void { @@ -260,7 +261,7 @@ class session_redis_test extends \advanced_testcase { $this->assertTrue($sess2->close()); // Read the session again to ensure locking did what it should. - $this->assertSessionNoLocks(); + $this->assert_session_no_locks(); } public function test_multiple_sessions_work_with_a_single_instance(): void { @@ -279,7 +280,7 @@ class session_redis_test extends \advanced_testcase { $this->assertTrue($sess->destroy('sess2')); $this->assertTrue($sess->close()); - $this->assertSessionNoLocks(); + $this->assert_session_no_locks(); $this->assertTrue($sess->close()); } @@ -299,12 +300,14 @@ class session_redis_test extends \advanced_testcase { $this->assertFalse($sess->session_exists('sess1'), 'Session should be destroyed.'); } - public function test_kill_sessions_removes_the_session_from_redis(): void { + public function test_destroy_removes_the_session_from_redis(): void { global $DB; $sess = new \core\session\redis(); $sess->init(); + $mockhandler = new mock_handler(); + $this->assertTrue($sess->open('Not used', 'Not used')); $this->assertTrue($sess->write('sess1', 'DATA')); $this->assertTrue($sess->write('sess2', 'DATA')); @@ -316,22 +319,27 @@ class session_redis_test extends \advanced_testcase { $sessiondata->timemodified = time(); $sessiondata->sid = 'sess1'; - $DB->insert_record('sessions', $sessiondata); + $mockhandler->add_test_session($sessiondata); $sessiondata->sid = 'sess2'; - $DB->insert_record('sessions', $sessiondata); + $mockhandler->add_test_session($sessiondata); $sessiondata->sid = 'sess3'; - $DB->insert_record('sessions', $sessiondata); + $mockhandler->add_test_session($sessiondata); $this->assertNotEquals('', $sess->read('sess1')); - $sess->kill_session('sess1'); + $sess->destroy('sess1'); $this->assertEquals('', $sess->read('sess1')); $this->assertEmpty($this->redis->keys($this->keyprefix.'sess1.lock')); - $sess->kill_all_sessions(); + $sess->destroy_all(); - $this->assertEquals(3, $DB->count_records('sessions'), 'Moodle handles session database, plugin must not change it.'); - $this->assertSessionNoLocks(); + $mockhandler = new mock_handler(); + $this->assertEquals( + 3, + $mockhandler->count_sessions(), + 'Moodle handles session database, plugin must not change it.', + ); + $this->assert_session_no_locks(); $this->assertEmpty($this->redis->keys($this->keyprefix.'*'), 'There should be no session data left.'); } @@ -360,7 +368,7 @@ class session_redis_test extends \advanced_testcase { /** * Assert that we don't have any session locks in Redis. */ - protected function assertSessionNoLocks() { + protected function assert_session_no_locks(): void { $this->assertEmpty($this->redis->keys($this->keyprefix.'*.lock')); } @@ -375,4 +383,208 @@ class session_redis_test extends \advanced_testcase { $this->assertEquals($CFG->session_redis_encrypt, $prop->getValue($sess)); } + + /** + * Test the get maxlifetime method. + */ + public function test_get_maxlifetime(): void { + global $CFG; + + // Set the timeout to something known for the test. + set_config('sessiontimeout', 100); + + // Generate a test user. + $user = $this->getDataGenerator()->create_user(); + + // Create a new redis session object. + $session = new \core\session\redis(); + $session->init(); + + // The get_maxlifetime is private, so we need to use reflection to access it. + $method = new \ReflectionMethod(\core\session\redis::class, 'get_maxlifetime'); + + // Test guest timeout, which should be longer. + $result = $method->invoke($session, $CFG->siteguest); + $this->assertEquals(500, $result); + + // Test first access timeout. + $result = $method->invoke($session, 0, true); + $this->assertEquals(180, $result); + + // Test with a real user. + $result = $method->invoke($session, $user->id); + $this->assertEquals(180, $result); + + } + + /** + * Test the add session method. + */ + public function test_add_session(): void { + + // Set the timeout to something known for the test. + set_config('sessiontimeout', 100); + + // Generate a test user. + $user = $this->getDataGenerator()->create_user(); + + // Create a new redis session object. + $session = new \core\session\redis(); + $session->init(); + + // Create two sessions for the user. + session_id('id1'); + $session1data = $session->add_session($user->id); + session_id('id2'); + $session2data = $session->add_session($user->id); + + $session1 = $session->get_session_by_sid('id1'); + $session2 = $session->get_session_by_sid('id2'); + + // Assert that the sessions were created and have expected data. + $this->assertEqualsCanonicalizing((array)$session1data, (array)$session1); + $this->assertEqualsCanonicalizing((array)$session2data, (array)$session2); + + // Check that the session hash has a ttl set. + $this->assertGreaterThan(-1, $this->redis->ttl($this->keyprefix . 'session_id1')); + + // Check that the session ttl is less or equal to what we set it. + $this->assertLessThanOrEqual(180, $this->redis->ttl($this->keyprefix . 'session_id1')); + + } + + /** + * Test writing session data. + */ + public function test_write(): void { + // Set the timeout to something known for the test. + set_config('sessiontimeout', 100); + + // Generate a test user. + $user = $this->getDataGenerator()->create_user(); + + // Create a new redis session object. + $session = new \core\session\redis(); + $session->init(); + + // Create two sessions for the user. + session_id('id1'); + $session->add_session($user->id); + session_id('id2'); + $session->add_session($user->id); + + $testdata = 'some test data'; + + // Write some data to the store. + $result = $session->write('id2', $testdata); + + // Check that the write was successful. + $this->assertTrue($result); + + // Check that the data was written to the store. + $getdata = $this->redis->hget($this->keyprefix . 'session_id2', 'sessdata'); + $this->assertStringContainsString($testdata, $getdata); + } + + /** + * Test reading session data. + */ + public function test_read(): void { + // Set the timeout to something known for the test. + set_config('sessiontimeout', 100); + + // Generate a test user. + $user = $this->getDataGenerator()->create_user(); + + // Create a new redis session object. + $session = new \core\session\redis(); + $session->init(); + + // Create two sessions for the user. + session_id('id1'); + $session->add_session($user->id); + session_id('id2'); + $session->add_session($user->id); + + $testdata = 'some test data'; + + // Write some session data to the store. + $session->write('id2', $testdata); + + // Read the session data. + $result = $session->read('id2'); + + // Check that the read was successful. + $this->assertEquals($result, $testdata); + + } + + /** + * Test updating a session. + */ + public function test_update_session(): void { + // Set the timeout to something known for the test. + set_config('sessiontimeout', 100); + + // Generate a test user. + $user = $this->getDataGenerator()->create_user(); + + // Create a new redis session object. + $session = new \core\session\redis(); + $session->init(); + + // Create two sessions for the user. + session_id('id1'); + $session->add_session($user->id); + session_id('id2'); + $sessiondata = $session->add_session($user->id); + + // Update the session data. + $sessiondata->lastip = '8.8.8.8'; + $session->update_session($sessiondata); + + // Check the value was updated. + $updatedsession = $session->get_session_by_sid('id2'); + $this->assertEquals('8.8.8.8', $updatedsession->lastip); + + // Test session update when userid is not set, should not error. + unset($sessiondata->userid); + $session->update_session($sessiondata); + $this->assertDebuggingNotCalled(); + } + + /** + * Test destroying a session by auth plugin. + */ + public function test_destroy_by_auth_plugin(): void { + // Create test users. + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(['auth' => 'db']); + + // Create a new redis session object. + $session = new \core\session\redis(); + $session->init(); + + // Create sessions for the users. + session_id('id1'); + $session1data = $session->add_session($user1->id); + session_id('id2'); + $session2data = $session->add_session($user2->id); + + $session1 = $session->get_session_by_sid('id1'); + $session2 = $session->get_session_by_sid('id2'); + + // Assert that the sessions were created and have expected data. + $this->assertEqualsCanonicalizing((array) $session1data, (array) $session1); + $this->assertEqualsCanonicalizing((array) $session2data, (array) $session2); + + // Destroy the session by auth plugin. + $session->destroy_by_auth_plugin('manual'); + + // Check that the session was destroyed. + $this->assertFalse($session->session_exists('id1')); + + // Check the session with db auth plugin was not destroyed. + $this->assertTrue($session->session_exists('id2')); + } } diff --git a/login/change_password.php b/login/change_password.php index 98d107d8ad1..d378092b316 100644 --- a/login/change_password.php +++ b/login/change_password.php @@ -119,7 +119,7 @@ if ($mform->is_cancelled()) { // Log out all other sessions if mandated by admin, or if set by the user. if (!empty($CFG->passwordchangelogout) || !empty($data->logoutothersessions)) { - \core\session\manager::kill_user_sessions($USER->id, session_id()); + \core\session\manager::destroy_user_sessions($USER->id, session_id()); } if (!empty($data->signoutofotherservices)) { diff --git a/login/lib.php b/login/lib.php index 9aaf2bfad6d..8c879975ac1 100644 --- a/login/lib.php +++ b/login/lib.php @@ -288,7 +288,7 @@ function core_login_process_password_set($token) { } user_add_password_history($user->id, $data->password); if (!empty($CFG->passwordchangelogout) || !empty($data->logoutothersessions)) { - \core\session\manager::kill_user_sessions($user->id, session_id()); + \core\session\manager::destroy_user_sessions($user->id, session_id()); } // Reset login lockout (if present) before a new password is set. login_unlock_account($user); diff --git a/report/usersessions/locallib.php b/report/usersessions/locallib.php index 02f6b5ae313..7f29ff65e5d 100644 --- a/report/usersessions/locallib.php +++ b/report/usersessions/locallib.php @@ -77,15 +77,15 @@ function report_usersessions_format_ip($ip) { * @param int $id * @return void */ -function report_usersessions_kill_session($id) { - global $DB, $USER; +function report_usersessions_kill_session(int $id): void { + global $USER; - $session = $DB->get_record('sessions', array('id' => $id, 'userid' => $USER->id), 'id, sid'); + $sessions = \core\session\manager::get_sessions_by_userid($USER->id); + $filteredsessions = array_filter($sessions, fn ($session) => $session->id === $id); - if (!$session or $session->sid === session_id()) { - // Do not delete the current session! - return; + foreach ($filteredsessions as $session) { + if ($session->sid !== session_id()) { + \core\session\manager::destroy($session->sid); + } } - - \core\session\manager::kill_session($session->sid); } diff --git a/report/usersessions/user.php b/report/usersessions/user.php index e295d901269..aa3657f673d 100644 --- a/report/usersessions/user.php +++ b/report/usersessions/user.php @@ -64,7 +64,7 @@ if ($delete && confirm_sesskey()) { // Delete all sessions except current. if ($deleteall && confirm_sesskey()) { - \core\session\manager::kill_user_sessions($USER->id, session_id()); + \core\session\manager::destroy_user_sessions($USER->id, session_id()); redirect( url: $PAGE->url, message: get_string('logoutothersessionssuccess', 'report_usersessions'), @@ -82,15 +82,13 @@ echo $OUTPUT->header(); echo $OUTPUT->heading(get_string('mysessions', 'report_usersessions')); $data = array(); -$sql = "SELECT id, timecreated, timemodified, firstip, lastip, sid - FROM {sessions} - WHERE userid = :userid - ORDER BY timemodified DESC"; -$params = array('userid' => $USER->id, 'sid' => session_id()); - -$sessions = $DB->get_records_sql($sql, $params); +$sessions = \core\session\manager::get_sessions_by_userid($USER->id); +// Order records by timemodified DESC. +usort($sessions, function($a, $b){ + return $b->timemodified <=> $a->timemodified; +}); foreach ($sessions as $session) { - if ($session->sid === $params['sid']) { + if ($session->sid === session_id()) { $lastaccess = get_string('thissession', 'report_usersessions'); $deletelink = ''; diff --git a/user/classes/privacy/provider.php b/user/classes/privacy/provider.php index d3e0feb43ca..705392115bb 100644 --- a/user/classes/privacy/provider.php +++ b/user/classes/privacy/provider.php @@ -294,7 +294,10 @@ class provider implements // Delete user course requests. $DB->delete_records('course_request', ['requester' => $userid]); // Delete sessions. - $DB->delete_records('sessions', ['userid' => $userid]); + $sessions = \core\session\manager::get_sessions_by_userid($userid); + foreach ($sessions as $session) { + \core\session\manager::destroy($session->sid); + } // Do I delete user preferences? Seems like the right place to do it. $DB->delete_records('user_preferences', ['userid' => $userid]); @@ -528,7 +531,7 @@ class provider implements protected static function export_user_session_data(int $userid, \context $context) { global $DB, $SESSION; - $records = $DB->get_records('sessions', ['userid' => $userid]); + $records = \core\session\manager::get_sessions_by_userid($userid); if (!empty($records)) { $sessiondata = (object) array_map(function($record) { return [ diff --git a/user/editadvanced.php b/user/editadvanced.php index 3d8795ebfef..b8065ae7fa6 100644 --- a/user/editadvanced.php +++ b/user/editadvanced.php @@ -233,7 +233,7 @@ if ($userform->is_cancelled()) { if (!empty($CFG->passwordchangelogout)) { // We can use SID of other user safely here because they are unique, // the problem here is we do not want to logout admin here when changing own password. - \core\session\manager::kill_user_sessions($usernew->id, session_id()); + \core\session\manager::destroy_user_sessions($usernew->id, session_id()); } if (!empty($usernew->signoutofotherservices)) { webservice::delete_user_ws_tokens($usernew->id); @@ -243,7 +243,7 @@ if ($userform->is_cancelled()) { // Force logout if user just suspended. if (isset($usernew->suspended) and $usernew->suspended and !$user->suspended) { - \core\session\manager::kill_user_sessions($user->id); + \core\session\manager::destroy_user_sessions($user->id); } } diff --git a/user/externallib.php b/user/externallib.php index c0add4cb15d..6713b6324ee 100644 --- a/user/externallib.php +++ b/user/externallib.php @@ -659,7 +659,7 @@ class core_user_external extends \core_external\external_api { useredit_update_user_preference($userpref); } if (isset($user['suspended']) and $user['suspended']) { - \core\session\manager::kill_user_sessions($user['id']); + \core\session\manager::destroy_user_sessions($user['id']); } $transaction->allow_commit(); diff --git a/user/tests/privacy/provider_test.php b/user/tests/privacy/provider_test.php index 21b5d2564ab..a7be316486b 100644 --- a/user/tests/privacy/provider_test.php +++ b/user/tests/privacy/provider_test.php @@ -13,33 +13,30 @@ // // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -/** - * Privacy tests for core_user. - * - * @package core_user - * @category test - * @copyright 2018 Adrian Greeve - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ + namespace core_user\privacy; -defined('MOODLE_INTERNAL') || die(); -global $CFG; - -use \core_privacy\tests\provider_testcase; -use \core_user\privacy\provider; -use \core_privacy\local\request\approved_userlist; -use \core_privacy\local\request\transform; - -require_once($CFG->dirroot . "/user/lib.php"); +use core\tests\session\mock_handler; +use core_privacy\tests\provider_testcase; +use core_privacy\local\request\approved_userlist; +use core_privacy\local\request\transform; +use core_user\privacy\provider; /** * Unit tests for core_user. * + * @package core_user * @copyright 2018 Adrian Greeve * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \core_user\privacy\provider */ -class provider_test extends provider_testcase { +final class provider_test extends provider_testcase { + public static function setUpBeforeClass(): void { + global $CFG; + parent::setUpBeforeClass(); + + require_once($CFG->dirroot . "/user/lib.php"); + } /** * Check that context information is returned correctly. @@ -475,7 +472,8 @@ class provider_test extends provider_testcase { 'firstip' => '0.0.0.0', 'lastip' => '0.0.0.0' ]; - $DB->insert_record('sessions', $usersessions); + $mockhandler = new mock_handler(); + $mockhandler->add_test_session($usersessions); } /** diff --git a/webservice/externallib.php b/webservice/externallib.php index cd76c808b22..b9389ff58b8 100644 --- a/webservice/externallib.php +++ b/webservice/externallib.php @@ -213,7 +213,7 @@ class core_webservice_external extends \core_external\external_api { $siteinfo['limitconcurrentlogins'] = (int) $CFG->limitconcurrentlogins; if (!empty($CFG->limitconcurrentlogins)) { // For performance, only when enabled. - $siteinfo['usersessionscount'] = $DB->count_records('sessions', ['userid' => $USER->id]); + $siteinfo['usersessionscount'] = count(\core\session\manager::get_sessions_by_userid($USER->id)); } $siteinfo['policyagreed'] = $USER->policyagreed; diff --git a/webservice/tests/externallib_test.php b/webservice/tests/externallib_test.php index 9dbce930e0b..3b41439d389 100644 --- a/webservice/tests/externallib_test.php +++ b/webservice/tests/externallib_test.php @@ -18,6 +18,7 @@ namespace core_webservice; use core_external\external_api; use externallib_advanced_testcase; +use core\tests\session\mock_handler; defined('MOODLE_INTERNAL') || die(); @@ -33,8 +34,8 @@ require_once($CFG->dirroot . '/webservice/tests/helpers.php'); * @copyright 2012 Paul Charsley * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class externallib_test extends externallib_advanced_testcase { - +final class externallib_test extends externallib_advanced_testcase { + #[\Override] public function setUp(): void { // Calling parent is good, always parent::setUp(); @@ -190,7 +191,9 @@ class externallib_test extends externallib_advanced_testcase { $record->firstip = $record->lastip = '10.0.0.1'; $record->sid = md5('hokus1'); $record->timecreated = time(); - $DB->insert_record('sessions', $record); + + $mockhandler = new mock_handler(); + $mockhandler->add_test_session($record); $siteinfo = \core_webservice_external::get_site_info(); $siteinfo = external_api::clean_returnvalue(\core_webservice_external::get_site_info_returns(), $siteinfo);