Skip to content

Commit

Permalink
MDL-65856 session: UX review of session timeout
Browse files Browse the repository at this point in the history 
Add new setting 'sessiontimeoutwarning', gives logged in user ability to extend session when there is no activity.
  • Loading branch information
@hdagheda
hdagheda committed Dec 20, 2020
1 parent 4ec279a commit 9c5dc8f
Show file tree
Hide file tree
Showing 9 changed files with 151 additions and 55 deletions.
16 changes: 16 additions & 0 deletions admin/settings/server.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,22 @@
$temp->add(new admin_setting_configduration('sessiontimeout', new lang_string('sessiontimeout', 'admin'),
new lang_string('configsessiontimeout', 'admin'), 8 * 60 * 60));

$sessiontimeoutwarning = new admin_setting_configduration('sessiontimeoutwarning',
new lang_string('sessiontimeoutwarning', 'admin'),
new lang_string('configsessiontimeoutwarning', 'admin'), 20 * 60);

$sessiontimeoutwarning->set_validate_function(function(int $value): string {
global $CFG;
// Check sessiontimeoutwarning is less than sessiontimeout.
if ($CFG->sessiontimeout <= $value) {
return get_string('configsessiontimeoutwarningcheck', 'admin');
} else {
return '';
}
});

$temp->add($sessiontimeoutwarning);

$temp->add(new admin_setting_configtext('sessioncookie', new lang_string('sessioncookie', 'admin'),
new lang_string('configsessioncookie', 'admin'), '', PARAM_ALPHANUM));
$temp->add(new admin_setting_configtext('sessioncookiepath', new lang_string('sessioncookiepath', 'admin'),
Expand Down
3 changes: 3 additions & 0 deletions lang/en/admin.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,8 @@
$string['configsessioncookiedomain'] = 'This allows you to change the domain that the Moodle cookies are available from. This is useful for Moodle customisations (e.g. authentication or enrolment plugins) that need to share Moodle session information with a web application on another subdomain. <strong>WARNING: it is strongly recommended to leave this setting at the default (empty) - an incorrect value will prevent all logins to the site.</strong>';
$string['configsessioncookiepath'] = 'If you need to change where browsers send the Moodle cookies, you can change this setting to specify a subdirectory of your web site. Otherwise the default \'/\' should be fine.';
$string['configsessiontimeout'] = 'If people logged in to this site are idle for a long time (without loading pages) then they are automatically logged out (their session is ended). This variable specifies how long this time should be.';
$string['configsessiontimeoutwarning'] = 'If people logged in to this site are idle for a long time (without loading pages) then they are warned about their session is about to end. This variable specifies how long this time should be.';
$string['configsessiontimeoutwarningcheck'] = 'Session timeout warning must be less than session timeout';
$string['configshowicalsource'] = 'Show source information for iCal events';
$string['configshowcommentscount'] = 'Show comments count, it will cost one more query when display comments link';
$string['configshowsiteparticipantslist'] = 'All of these site students and site teachers will be listed on the site participants list. Who shall be allowed to see this site participants list?';
Expand Down Expand Up @@ -1191,6 +1193,7 @@
$string['sessioncookiepath'] = 'Cookie path';
$string['sessionhandling'] = 'Session handling';
$string['sessiontimeout'] = 'Timeout';
$string['sessiontimeoutwarning'] = 'Timeout Warning';
$string['settingdependenton'] = 'This setting may be hidden, based on the value of <strong>{$a}</strong>.';
$string['settingfileuploads'] = 'File uploading is required for normal operation, please enable it in PHP configuration.';
$string['settingmemorylimit'] = 'Insufficient memory detected, please set higher memory limit in PHP settings.';
Expand Down
1 change: 1 addition & 0 deletions lang/en/moodle.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1180,6 +1180,7 @@
<p>All you need to do is make up a username and password and use it in the form on this page!</p>
<p>If someone else has already chosen your username then you\'ll have to try again using a different username.</p>';
$string['loginto'] = 'Log in to {$a}';
$string['loginagain'] = 'Log in again';
$string['logout'] = 'Log out';
$string['logoutconfirm'] = 'Do you really want to log out?';
$string['logs'] = 'Logs';
Expand Down
38 changes: 38 additions & 0 deletions lib/adminlib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3814,6 +3814,8 @@ class admin_setting_configduration extends admin_setting {

/** @var int default duration unit */
protected $defaultunit;
/** @var callable|null Validation function */
protected $validatefunction = null;

/**
* Constructor
Expand All @@ -3837,6 +3839,36 @@ public function __construct($name, $visiblename, $description, $defaultsetting,
parent::__construct($name, $visiblename, $description, $defaultsetting);
}

/**
* Sets a validate function.
*
* The callback will be passed one parameter, the new setting value, and should return either
* an empty string '' if the value is OK, or an error message if not.
*
* @param callable|null $validatefunction Validate function or null to clear
* @since Moodle 3.10
*/
public function set_validate_function(?callable $validatefunction = null) {
$this->validatefunction = $validatefunction;
}

/**
* Validate the setting. This uses the callback function if provided; subclasses could override
* to carry out validation directly in the class.
*
* @param int $data New value being set
* @return string Empty string if valid, or error message text
* @since Moodle 3.10
*/
protected function validate_setting(int $data): string {
// If validation function is specified, call it now.
if ($this->validatefunction) {
return call_user_func($this->validatefunction, $data);
} else {
return '';
}
}

/**
* Returns selectable units.
* @static
Expand Down Expand Up @@ -3922,6 +3954,12 @@ public function write_setting($data) {
return get_string('errorsetting', 'admin');
}

// Validate the new setting.
$error = $this->validate_setting($seconds);
if ($error) {
return $error;
}

$result = $this->config_write($this->name, $seconds);
return ($result ? '' : get_string('errorsetting', 'admin'));
}
Expand Down
2 changes: 1 addition & 1 deletion lib/amd/build/network.min.js
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/amd/build/network.min.js.map
View file

Large diffs are not rendered by default.

111 changes: 72 additions & 39 deletions lib/amd/src/network.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,66 @@ define(['jquery', 'core/ajax', 'core/config', 'core/notification', 'core/str'],
var sessionTimeout = false;
// 1/10 of session timeout, max of 10 minutes.
var checkFrequency = Math.min((Config.sessiontimeout / 10), 600) * 1000;
// 1/5 of sessiontimeout.
var warningLimit = checkFrequency * 2;

// Check if sessiontimeoutwarning is set or double the checkFrequency.
var warningLimit = (Config.sessiontimeoutwarning > 0) ? (Config.sessiontimeoutwarning * 1000) : (checkFrequency * 2);
// First wait is minimum of remaining time or half of the session timeout.
var firstWait = (Config.sessiontimeoutwarning > 0) ?
Math.min((Config.sessiontimeout - Config.sessiontimeoutwarning) * 1000, checkFrequency * 5) : checkFrequency * 5;
/**
* The session time has expired - we can't extend it now.
* @param {Modal} modal
*/
var timeoutSessionExpired = function() {
var timeoutSessionExpired = function(modal) {
sessionTimeout = true;
warningDisplayed = false;
closeModal(modal);
displaySessionExpired();
};

/**
* Close modal - this relies on modal object passed from Notification.confirm.
*
* @param {Modal} modal
*/
var closeModal = function(modal) {
modal.destroy();
};

/**
* The session time has expired - we can't extend it now.
* @return {Promise}
*/
var displaySessionExpired = function() {
// Check again if its already extended before displaying session expired popup in case multiple tabs are open.
var request = {
methodname: 'core_session_time_remaining',
args: { }
};

return Ajax.call([request], true, true, true)[0].then(function(args) {
if (args.timeremaining * 1000 > warningLimit) {
return false;
} else {
return Str.get_strings([
{key: 'sessionexpired', component: 'error'},
{key: 'sessionerroruser', component: 'error'},
{key: 'loginagain', component: 'moodle'},
{key: 'cancel', component: 'moodle'}
]).then(function(strings) {
Notification.confirm(
strings[0], // Title.
strings[1], // Message.
strings[2], // Login Again.
strings[3], // Cancel.
function() {
location.reload();
return true;
}
);
return true;
}).catch(Notification.exception);
}
});
};

/**
Expand All @@ -55,23 +107,14 @@ define(['jquery', 'core/ajax', 'core/config', 'core/notification', 'core/str'],

if (sessionTimeout) {
// We timed out before we extended the session.
return Str.get_strings([
{key: 'sessionexpired', component: 'error'},
{key: 'sessionerroruser', component: 'error'}
]).then(function(strings) {
Notification.alert(
strings[0], // Title.
strings[1] // Message.
);
return true;
}).fail(Notification.exception);
return displaySessionExpired();
} else {
return Ajax.call([request], true, true, false, requestTimeout)[0].then(function() {
if (keepAliveFrequency > 0) {
setTimeout(touchSession, keepAliveFrequency);
}
return true;
}).fail(function() {
}).catch(function() {
Notification.alert('', keepAliveMessage);
});
}
Expand All @@ -88,53 +131,43 @@ define(['jquery', 'core/ajax', 'core/config', 'core/notification', 'core/str'],
methodname: 'core_session_time_remaining',
args: { }
};

sessionTimeout = false;
return Ajax.call([request], true, true, true)[0].then(function(args) {
if (args.userid <= 0) {
return false;
}
if (args.timeremaining < 0) {
Str.get_strings([
{key: 'sessionexpired', component: 'error'},
{key: 'sessionerroruser', component: 'error'}
]).then(function(strings) {
Notification.alert(
strings[0], // Title.
strings[1] // Message.
);
return true;
}).fail(Notification.exception);

} else if (args.timeremaining * 1000 < warningLimit && !warningDisplayed) {
// If we don't extend the session before the timeout - warn.
setTimeout(timeoutSessionExpired, args.timeremaining * 1000);
if (args.timeremaining <= 0) {
return displaySessionExpired();
} else if (args.timeremaining * 1000 <= warningLimit && !warningDisplayed) {
warningDisplayed = true;
Str.get_strings([
{key: 'norecentactivity', component: 'moodle'},
{key: 'sessiontimeoutsoon', component: 'moodle'},
{key: 'extendsession', component: 'moodle'},
{key: 'cancel', component: 'moodle'}
]).then(function(strings) {
Notification.confirm(
return Notification.confirm(
strings[0], // Title.
strings[1], // Message.
strings[2], // Extend session.
strings[3], // Cancel.
function() {
touchSession();
warningDisplayed = false;
// First wait is half the session timeout.
setTimeout(checkSession, checkFrequency * 5);
// First wait is minimum of remaining time or half of the session timeout.
setTimeout(checkSession, firstWait);
return true;
},
function() {
warningDisplayed = false;
// User has cancelled notification.
setTimeout(checkSession, checkFrequency);
}
);
return true;
}).fail(Notification.exception);
}).then(modal => {
// If we don't extend the session before the timeout - warn.
setTimeout(timeoutSessionExpired, args.timeremaining * 1000, modal);
return;
}).catch(Notification.exception);
} else {
setTimeout(checkSession, checkFrequency);
}
Expand All @@ -151,8 +184,8 @@ define(['jquery', 'core/ajax', 'core/config', 'core/notification', 'core/str'],
if (keepAliveFrequency > 0) {
setTimeout(touchSession, keepAliveFrequency);
} else {
// First wait is half the session timeout.
setTimeout(checkSession, checkFrequency * 5);
// First wait is minimum of remaining time or half of the session timeout.
setTimeout(checkSession, firstWait);
}
};

Expand Down
29 changes: 15 additions & 14 deletions lib/outputrequirementslib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,20 +319,21 @@ public function get_config_for_javascript(moodle_page $page, renderer_base $rend
}

$this->M_cfg = array(
'wwwroot' => $CFG->wwwroot,
'sesskey' => sesskey(),
'sessiontimeout' => $CFG->sessiontimeout,
'themerev' => theme_get_revision(),
'slasharguments' => (int)(!empty($CFG->slasharguments)),
'theme' => $page->theme->name,
'iconsystemmodule' => $iconsystem->get_amd_name(),
'jsrev' => $this->get_jsrev(),
'admin' => $CFG->admin,
'svgicons' => $page->theme->use_svg_icons(),
'usertimezone' => usertimezone(),
'contextid' => $contextid,
'langrev' => get_string_manager()->get_revision(),
'templaterev' => $this->get_templaterev()
'wwwroot' => $CFG->wwwroot,
'sesskey' => sesskey(),
'sessiontimeout' => $CFG->sessiontimeout,
'sessiontimeoutwarning' => $CFG->sessiontimeoutwarning,
'themerev' => theme_get_revision(),
'slasharguments' => (int)(!empty($CFG->slasharguments)),
'theme' => $page->theme->name,
'iconsystemmodule' => $iconsystem->get_amd_name(),
'jsrev' => $this->get_jsrev(),
'admin' => $CFG->admin,
'svgicons' => $page->theme->use_svg_icons(),
'usertimezone' => usertimezone(),
'contextid' => $contextid,
'langrev' => get_string_manager()->get_revision(),
'templaterev' => $this->get_templaterev()
);
if ($CFG->debugdeveloper) {
$this->M_cfg['developerdebug'] = true;
Expand Down
4 changes: 4 additions & 0 deletions lib/setup.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -801,6 +801,10 @@
if (empty($CFG->sessiontimeout)) {
$CFG->sessiontimeout = 8 * 60 * 60;
}
// Set sessiontimeoutwarning 20 minutes.
if (empty($CFG->sessiontimeoutwarning)) {
$CFG->sessiontimeoutwarning = 20 * 60;
}
\core\session\manager::start();

// Set default content type and encoding, developers are still required to use
Expand Down

0 comments on commit 9c5dc8f

Please sign in to comment.