MDL-65856 session: UX review of session timeout
authorHeena Agheda <heenaagheda@catalyst-au.net>
Thu, 26 Mar 2020 04:02:02 +0000 (15:02 +1100)
committerHeena Agheda <heenaagheda@catalyst-au.net>
Sun, 20 Dec 2020 21:43:21 +0000 (08:43 +1100)
Add new setting 'sessiontimeoutwarning', gives logged in user ability to extend session when there is no activity.

admin/settings/server.php
lang/en/admin.php
lang/en/moodle.php
lib/adminlib.php
lib/amd/build/network.min.js
lib/amd/build/network.min.js.map
lib/amd/src/network.js
lib/outputrequirementslib.php
lib/setup.php

index 11541bb..beb70f3 100644 (file)
@@ -73,6 +73,22 @@ if ($hassiteconfig) {
     $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'),
index 4e1f6c3..92781b5 100644 (file)
@@ -348,6 +348,8 @@ $string['configsessioncookie'] = 'This setting customises the name of the cookie
 $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?';
@@ -1191,6 +1193,7 @@ $string['sessioncookiedomain'] = 'Cookie domain';
 $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.';
index 8fbcca4..95c040d 100644 (file)
@@ -1180,6 +1180,7 @@ $string['loginstepsnone'] = '<p>Hi!</p>
 <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';
index ff74c9a..1177b59 100644 (file)
@@ -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
@@ -3837,6 +3839,36 @@ class admin_setting_configduration extends admin_setting {
         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
@@ -3922,6 +3954,12 @@ class admin_setting_configduration extends admin_setting {
             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'));
     }
index 81e4444..680602f 100644 (file)
Binary files a/lib/amd/build/network.min.js and b/lib/amd/build/network.min.js differ
index b2204e1..df849d1 100644 (file)
Binary files a/lib/amd/build/network.min.js.map and b/lib/amd/build/network.min.js.map differ
index 5e21a6a..fcf1a0b 100644 (file)
@@ -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);
+            }
+        });
     };
 
     /**
@@ -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);
             });
         }
@@ -88,27 +131,14 @@ 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'},
@@ -116,7 +146,7 @@ define(['jquery', 'core/ajax', 'core/config', 'core/notification', 'core/str'],
                     {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.
@@ -124,17 +154,20 @@ define(['jquery', 'core/ajax', 'core/config', 'core/notification', 'core/str'],
                         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);
             }
@@ -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);
         }
     };
 
index 7bca2a7..d18b9f5 100644 (file)
@@ -319,20 +319,21 @@ class page_requirements_manager {
             }
 
             $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;
index 1326f13..210bdee 100644 (file)
@@ -801,6 +801,10 @@ if (CLI_SCRIPT) {
 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