Commit 0913742c authored by Aaron Wells's avatar Aaron Wells Committed by Robert Lyon

Bug 1590293: Correcting inconsistencies in session expiration

1. Add some documentation to session.php explaining what
the session.gc_maxlifetime ini setting does.

2. If we can't access $CFG->session_timeout, use a timeout of
an hour instead of the PHP default of 24 minutes.

3. Limit $CFG->session_timeout to 30 days, because we're already
enforcing that limit in session.php

4. Add "usr_session.mtime" column so that we can delete old sessions
based on inactivity instead of creation date.

5. Make the cron delete old session files as soon as they've expired,
rather than padding that an additional two days.

Change-Id: I9da2b26217774566b1131e997724359715edb2fe
behatnotneeded: Covered by existing tests
parent 7f697c51
......@@ -339,7 +339,8 @@ $siteoptionform = array(
'title' => get_string('sessionlifetime', 'admin'),
'description' => get_string('sessionlifetimedescription', 'admin'),
'defaultvalue' => get_config('session_timeout') / 60,
'rules' => array('integer' => true, 'minvalue' => 1, 'maxvalue' => 10000000),
// Largest amount allowed is 30 days.
'rules' => array('integer' => true, 'minvalue' => 1, 'maxvalue' => (30 * 24 * 60)),
'help' => true,
'disabled' => in_array('session_timeout', $OVERRIDDEN),
),
......
......@@ -1897,8 +1897,8 @@ function auth_handle_institution_expiries() {
function auth_remove_old_session_files() {
$basedir = get_config('sessionpath');
// delete sessions older than the session timeout plus 2 days
$mintime = time() - get_config('session_timeout') - 2 * 24 * 60 * 60;
// delete sessions older than the session timeout
$mintime = time() - get_config('session_timeout');
// Session files are stored in a three tier md5sum layout
// The actual files are stored in the third directory
......@@ -1925,8 +1925,8 @@ function auth_remove_old_session_files() {
}
}
}
// Throw away records of old login sessions. Should check whether any are still alive.
delete_records_select('usr_session', 'ctime < ?', array(db_format_timestamp(time() - 86400 * 30)));
// Delete database records of expired login sessions.
delete_records_select('usr_session', 'mtime < ?', array(db_format_timestamp($mintime - get_config('accesstimeupdatefrequency'))));
}
/**
......
......@@ -54,10 +54,22 @@ class Session {
ini_set('session.use_only_cookies', true);
ini_set('session.hash_bits_per_character', 4);
ini_set('session.gc_divisor', 1000);
// session timeout must not exceed 30 days
if (get_config('session_timeout')) {
ini_set('session.gc_maxlifetime', min(get_config('session_timeout'), 60 * 60 * 24 * 30));
// Limit session timeout to 30 days.
$session_timeout = min(get_config('session_timeout'), 60 * 60 * 24 * 30);
}
else {
// If session was started up by an error message before the database was initiated,
// then fall back to a default session timeout of 1 hour.
$session_timeout = 60 * 60;
}
// Note: session.gc_maxlifetime is not the main way login session expiry is enforced.
// We do that by looking at usr.last_access, in htdocs/auth/user.php.
// And if you're using the default PHP file session handler with depthdir 3, cleanup
// of old session files is actually handled by the Mahara cron task auth_remove_old_session_files.
ini_set('session.gc_maxlifetime', $session_timeout);
ini_set('session.use_trans_sid', false);
ini_set('session.hash_function', 'sha256'); // stronger hash functions are sha384 and sha512
if (version_compare(PHP_VERSION, '5.5.2') > 0) {
......
......@@ -1618,6 +1618,7 @@ class LiveUser extends User {
* continuing
*/
public function renew() {
global $SESSION, $CFG;
$time = time();
$this->set('logout_time', $time + get_config('session_timeout'));
$oldlastaccess = $this->get('lastaccess');
......@@ -1625,13 +1626,15 @@ class LiveUser extends User {
// prevent updating before this time has expired.
// If it is set to zero, we always update the accesstime.
$accesstimeupdatefrequency = get_config('accesstimeupdatefrequency');
if ($accesstimeupdatefrequency == 0) {
$this->set('lastaccess', $time);
$this->commit();
}
else if ($oldlastaccess + $accesstimeupdatefrequency < $time) {
if (
$accesstimeupdatefrequency == 0
|| $oldlastaccess + $accesstimeupdatefrequency < $time
) {
$this->set('lastaccess', $time);
$this->commit();
if ($CFG->version >= 2016060800) {
set_field('usr_session', 'mtime', db_format_timestamp($time), 'session', $SESSION->session_id());
}
}
}
......@@ -1832,6 +1835,7 @@ class LiveUser extends User {
'usr' => $this->get('id'),
'session' => $sessionid,
'ctime' => db_format_timestamp(time()),
'mtime' => db_format_timestamp(time()),
));
}
......
......@@ -226,6 +226,7 @@
<FIELD NAME="usr" TYPE="int" LENGTH="10" NOTNULL="true" />
<FIELD NAME="session" TYPE="char" LENGTH="255" NOTNULL="true" />
<FIELD NAME="ctime" TYPE="datetime" NOTNULL="true"/>
<FIELD NAME="mtime" TYPE="datetime" NOTNULL="false"/>
</FIELDS>
<KEYS>
<KEY NAME="primary" TYPE="primary" FIELDS="session" />
......
......@@ -4455,5 +4455,22 @@ function xmldb_core_upgrade($oldversion=0) {
}
}
if ($oldversion < 2016033110) {
log_debug('Add an "mtime" field to usr_session table');
$table = new XMLDBTable('usr_session');
$field = new XMLDBField('mtime');
$field->setAttributes(XMLDB_TYPE_DATETIME);
if (!field_exists($table, $field)) {
add_field($table, $field);
// Fill in starting value for existing sessions.
execute_sql('UPDATE {usr_session} SET mtime=ctime');
}
log_debug('Limit session_timeout to 30 days.');
if (get_config('session_timeout') > 60 * 60 * 24 * 30) {
set_config('session_timeout', 60 * 60 * 24 * 30);
}
}
return $status;
}
......@@ -16,7 +16,7 @@ $config = new stdClass();
// See https://wiki.mahara.org/wiki/Developer_Area/Version_Numbering_Policy
// For upgrades on stable branches, increment the version by one. On master, use the date.
$config->version = 2016033109;
$config->version = 2016033110;
$config->series = '16.04';
$config->release = '16.04.1testing';
$config->minupgradefrom = 2008040200;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment