MDL-46561 session: use full session validation in \core\session\manager::session_exists()

auth/upgrade.txt

@@ -1,6 +1,11 @@
 This files describes API changes in /auth/* - plugins,
 information provided here is intended especially for developers.
 
+=== 2.8 ===
+
+* \core\session\manager::session_exists() now verifies the session is active
+  instead of only checking the session data is present in low level session handler
+
 === 2.7 ===
 
 * If you are returning a url in method change_password_url() from config, please make sure it is set before trying to use it.

lib/classes/session/database.php

@@ -82,19 +82,16 @@ class database extends handler {
     }
 
     /**
-     * Check for existing session with id $sid.
+     * Check the backend contains data for this session id.
      *
-     * Note: this verifies the storage backend only, not the actual session records.
+     * Note: this is intended to be called from manager::session_exists() only.
      *
      * @param string $sid
      * @return bool true if session found.
      */
     public function session_exists($sid) {
-        try {
-            return $this->database->record_exists('sessions', array('sid'=>$sid, 'state'=>0));
-        } catch (\dml_exception $ex) {
-            return false;
-        }
+        // It was already checked in the calling code that the record in sessions table exists.
+        return true;
     }
 
     /**

lib/classes/session/file.php

@@ -76,9 +76,9 @@ class file extends handler {
     }
 
     /**
-     * Check for existing session with id $sid.
+     * Check the backend contains data for this session id.
      *
-     * Note: this verifies the storage backend only, not the actual session records.
+     * Note: this is intended to be called from manager::session_exists() only.
      *
      * @param string $sid
      * @return bool true if session found.

lib/classes/session/handler.php

@@ -48,9 +48,9 @@ abstract class handler {
     public abstract function init();
 
     /**
-     * Check for existing session with id $sid.
+     * Check the backend contains data for this session id.
      *
-     * Note: this verifies the storage backend only, not the actual session records.
+     * Note: this is intended to be called from manager::session_exists() only.
      *
      * @param string $sid
      * @return bool true if session found.

lib/classes/session/manager.php

@@ -524,12 +524,34 @@ class manager {
     /**
      * Does the PHP session with given id exist?
      *
-     * Note: this does not actually verify the presence of sessions record.
+     * The session must exist both in session table and actual
+     * session backend and the session must not be timed out.
+     *
+     * Timeout evaluation is simplified, the auth hooks are not executed.
      *
      * @param string $sid
      * @return bool
      */
     public static function session_exists($sid) {
+        global $DB, $CFG;
+
+        if (empty($CFG->version)) {
+            // Not installed yet, do not try to access database.
+            return false;
+        }
+
+        // Note: add sessions->state checking here if it gets implemented.
+        if (!$record = $DB->get_record('sessions', array('sid' => $sid), 'id, userid, timemodified')) {
+            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) {
+            return false;
+        }
+
+        // There is no need the existence of handler storage in public API.
         self::load_handler();
         return self::$handler->session_exists($sid);
     }

lib/classes/session/memcache.php

@@ -111,9 +111,9 @@ class memcache extends handler {
     }
 
     /**
-     * Checks for existing session with given id.
+     * Check the backend contains data for this session id.
      *
-     * Note: this verifies the storage backend only, not the actual session records.
+     * Note: this is intended to be called from manager::session_exists() only.
      *
      * @param string $sid PHP session ID
      * @return bool true if session found.

lib/classes/session/memcached.php

@@ -125,9 +125,9 @@ class memcached extends handler {
     }
 
     /**
-     * Check for existing session with id $sid.
+     * Check the backend contains data for this session id.
      *
-     * Note: this verifies the storage backend only, not the actual session records.
+     * Note: this is intended to be called from manager::session_exists() only.
      *
      * @param string $sid
      * @return bool true if session found.

lib/tests/session_manager_test.php

@@ -174,14 +174,48 @@ class core_session_manager_testcase extends advanced_testcase {
     }
 
     public function test_session_exists() {
-        global $CFG;
+        global $CFG, $DB;
         $this->resetAfterTest();
 
+        $this->assertFalse(\core\session\manager::session_exists('abc'));
+
+        $user = $this->getDataGenerator()->create_user();
+        $guest = guest_user();
+
         // The file handler is used by default, so let's fake the data somehow.
         $sid = md5('hokus');
         mkdir("$CFG->dataroot/sessions/", $CFG->directorypermissions, true);
         touch("$CFG->dataroot/sessions/sess_$sid");
 
+        $this->assertFalse(\core\session\manager::session_exists($sid));
+
+        $record = new stdClass();
+        $record->userid = 0;
+        $record->sid = $sid;
+        $record->timecreated = time();
+        $record->timemodified = $record->timecreated;
+        $record->id = $DB->insert_record('sessions', $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);
+
+        $this->assertTrue(\core\session\manager::session_exists($sid));
+
+        $record->userid = $guest->id;
+        $DB->update_record('sessions', $record);
+
+        $this->assertTrue(\core\session\manager::session_exists($sid));
+
+        $record->userid = $user->id;
+        $DB->update_record('sessions', $record);
+
+        $this->assertFalse(\core\session\manager::session_exists($sid));
+
+        $CFG->sessiontimeout = $CFG->sessiontimeout + 3000;
+
         $this->assertTrue(\core\session\manager::session_exists($sid));
     }