diff --git a/lib/classes/session/manager.php b/lib/classes/session/manager.php index a84424c6945..338f190236c 100644 --- a/lib/classes/session/manager.php +++ b/lib/classes/session/manager.php @@ -260,7 +260,27 @@ class manager { // Set configuration. session_name($sessionname); - session_set_cookie_params(0, $CFG->sessioncookiepath, $CFG->sessioncookiedomain, $cookiesecure, $CFG->cookiehttponly); + + if (version_compare(PHP_VERSION, '7.3.0', '>=')) { + $sessionoptions = [ + 'lifetime' => 0, + 'path' => $CFG->sessioncookiepath, + 'domain' => $CFG->sessioncookiedomain, + 'secure' => $cookiesecure, + 'httponly' => $CFG->cookiehttponly, + ]; + + if (self::should_use_samesite_none()) { + // If $samesite is empty, we don't want there to be any SameSite attribute. + $sessionoptions['samesite'] = 'None'; + } + + session_set_cookie_params($sessionoptions); + } else { + // Once PHP 7.3 becomes our minimum, drop this in favour of the alternative call to session_set_cookie_params above, + // as that does not require a hack to work with same site settings on cookies. + session_set_cookie_params(0, $CFG->sessioncookiepath, $CFG->sessioncookiedomain, $cookiesecure, $CFG->cookiehttponly); + } ini_set('session.use_trans_sid', '0'); ini_set('session.use_only_cookies', '1'); ini_set('session.hash_function', '0'); // For now MD5 - we do not have room for sha-1 in sessions table. @@ -420,6 +440,8 @@ class manager { if ($timedout) { $_SESSION['SESSION']->has_timed_out = true; } + + self::append_samesite_cookie_attribute(); } /** @@ -487,6 +509,61 @@ class manager { // Setup $USER object. self::set_user($user); + self::append_samesite_cookie_attribute(); + } + + /** + * Returns a valid setting for the SameSite cookie attribute. + * + * @return string The desired setting for the SameSite attribute on the cookie. Empty string indicates the SameSite attribute + * should not be set at all. + */ + private static function should_use_samesite_none() { + // We only want None or no attribute at this point. When we have cookie handling compatible with Lax, + // we can look at checking a setting. + + // Browser support for none is not consistent yet. There are known issues with Safari, and IE11. + // Things are stablising, however as they're not stable yet we will deal specifically with the version of chrome + // that introduces a default of lax, setting it to none for the current version of chrome (2 releases before the change). + // We also check you are using secure cookies and HTTPS because if you are not running over HTTPS + // then setting SameSite=None will cause your session cookie to be rejected. + if (\core_useragent::is_chrome() && \core_useragent::check_chrome_version('78') && is_moodle_cookie_secure()) { + return true; + } + return false; + } + + /** + * Conditionally append the SameSite attribute to the session cookie if necessary. + * + * Contains a hack for versions of PHP lower than 7.3 as there is no API built into PHP cookie API + * for adding the SameSite setting. + * + * This won't change the Set-Cookie headers if: + * - PHP 7.3 or higher is being used. That already adds the SameSite attribute without any hacks. + * - If the samesite setting is empty. + * - If the samesite setting is None but the browser is not compatible with that setting. + */ + private static function append_samesite_cookie_attribute() { + if (version_compare(PHP_VERSION, '7.3.0', '>=')) { + // This hack is only necessary if we weren't able to set the samesite flag via the session_set_cookie_params API. + return; + } + + if (!self::should_use_samesite_none()) { + return; + } + + $cookies = headers_list(); + header_remove('Set-Cookie'); + $setcookiesession = 'Set-Cookie: ' . session_name() . '='; + + foreach ($cookies as $cookie) { + if (strpos($cookie, $setcookiesession) === 0) { + $cookie .= '; SameSite=None'; + } + header($cookie, false); + } } /** @@ -523,6 +600,8 @@ class manager { self::add_session_record($_SESSION['USER']->id); // Do not use $USER here because it may not be set up yet. session_write_close(); self::$sessionactive = false; + + self::append_samesite_cookie_attribute(); } /** diff --git a/mod/scorm/datamodels/aicc.js b/mod/scorm/datamodels/aicc.js index cc103bbabab..bf84bd05389 100644 --- a/mod/scorm/datamodels/aicc.js +++ b/mod/scorm/datamodels/aicc.js @@ -508,6 +508,7 @@ function AICCapi(def, cmiobj, scormauto, cfgwwwroot, scormid, scoid, attempt, vi } function StoreData(data,storetotaltime) { + var datastring = ''; if (storetotaltime) { if (cmi.core.lesson_mode == 'normal') { if (cmi.core.credit == 'credit') { @@ -531,9 +532,13 @@ function AICCapi(def, cmiobj, scormauto, cfgwwwroot, scormid, scoid, attempt, vi datastring = CollectData(data,'cmi'); } - //popupwin(datastring); var myRequest = NewHttpReq(); - result = DoRequest(myRequest,datamodelurl,datamodelurlparams + datastring); + var result = DoRequest(myRequest, datamodelurl, datamodelurlparams + datastring); + + if (result === false) { + return false; + } + results = String(result).split('\n'); errorCode = results[1]; return results[0]; diff --git a/mod/scorm/datamodels/scorm_12.js b/mod/scorm/datamodels/scorm_12.js index a6d035859b1..4d540ba897f 100644 --- a/mod/scorm/datamodels/scorm_12.js +++ b/mod/scorm/datamodels/scorm_12.js @@ -619,6 +619,7 @@ function SCORMapi1_2(def, cmiobj, cmiint, cmistring256, cmistring4096, scormdebu } function StoreData(data,storetotaltime) { + var datastring = ''; if (storetotaltime) { if (cmi.core.lesson_status == 'not attempted') { cmi.core.lesson_status = 'completed'; @@ -647,7 +648,12 @@ function SCORMapi1_2(def, cmiobj, cmiint, cmistring256, cmistring4096, scormdebu var myRequest = NewHttpReq(); //alert('going to:' + "wwwroot) ?>/mod/scorm/datamodel.php" + "id=&a=&sesskey="+datastring); - result = DoRequest(myRequest,datamodelurl,datamodelurlparams + datastring); + var result = DoRequest(myRequest, datamodelurl, datamodelurlparams + datastring); + + if (result === false) { + return false; + } + results = String(result).split('\n'); errorCode = results[1]; return results[0]; diff --git a/mod/scorm/datamodels/scorm_13.js b/mod/scorm/datamodels/scorm_13.js index e3b02a4c106..e43521130ee 100644 --- a/mod/scorm/datamodels/scorm_13.js +++ b/mod/scorm/datamodels/scorm_13.js @@ -1227,7 +1227,11 @@ function SCORMapi1_3(def, cmiobj, cmiint, cmicommentsuser, cmicommentslms, scorm datastring += navrequest; var myRequest = NewHttpReq(); - result = DoRequest(myRequest, datamodelurl, datamodelurlparams + datastring); + var result = DoRequest(myRequest, datamodelurl, datamodelurlparams + datastring); + + if (result === false) { + return false; + } var results = String(result).split('\n'); if ((results.length > 2) && (navrequest != '')) { diff --git a/mod/scorm/module.js b/mod/scorm/module.js index b1df94ff049..0fd411a4361 100644 --- a/mod/scorm/module.js +++ b/mod/scorm/module.js @@ -460,9 +460,16 @@ M.mod_scorm.init = function(Y, nav_display, navposition_left, navposition_top, h if (scoes_nav[launch_sco].flow === 1) { var datastring = scoes_nav[launch_sco].url + '&function=scorm_seq_flow&request=backward'; result = scorm_ajax_request(M.cfg.wwwroot + '/mod/scorm/datamodels/sequencinghandler.php?', datastring); - mod_scorm_seq = encodeURIComponent(result); - result = Y.JSON.parse (result); - if (typeof result.nextactivity.id != undefined) { + + if (result === false) { + // Either the outcome was a failure, or we are unloading and simply just don't know + // what the outcome actually was. + result = {}; + } else { + result = Y.JSON.parse(result); + } + + if (typeof result.nextactivity !== 'undefined' && typeof result.nextactivity.id !== 'undefined') { var node = scorm_prev(scorm_tree_node.getSelectedNodes()[0]); if (node == null) { // Avoid use of TreeView for Navigation. @@ -492,8 +499,15 @@ M.mod_scorm.init = function(Y, nav_display, navposition_left, navposition_top, h if (scoes_nav[launch_sco].flow === 1) { var datastring = scoes_nav[launch_sco].url + '&function=scorm_seq_flow&request=forward'; result = scorm_ajax_request(M.cfg.wwwroot + '/mod/scorm/datamodels/sequencinghandler.php?', datastring); - mod_scorm_seq = encodeURIComponent(result); - result = Y.JSON.parse (result); + + if (result === false) { + // Either the outcome was a failure, or we are unloading and simply just don't know + // what the outcome actually was. + result = {}; + } else { + result = Y.JSON.parse(result); + } + if (typeof result.nextactivity !== 'undefined' && typeof result.nextactivity.id !== 'undefined') { var node = scorm_next(scorm_tree_node.getSelectedNodes()[0]); if (node === null) { diff --git a/mod/scorm/request.js b/mod/scorm/request.js index 190fe957003..4bea10b9c20 100644 --- a/mod/scorm/request.js +++ b/mod/scorm/request.js @@ -37,6 +37,34 @@ function NewHttpReq() { function DoRequest(httpReq,url,param) { + // If we are unloading, and we can use sendBeacon then do that, Chrome does not permit synchronous XHR requests on unload. + if (window.mod_scorm_is_window_closing && navigator && navigator.sendBeacon && FormData) { + // Ok, old API alert, the param is a URI encoded string. We need to split it and convert it to a supported format. + // I've chosen FormData and FormData.append as they are compatible with our supported browsers: + // - https://developer.mozilla.org/en-US/docs/Web/API/FormData/FormData + // - https://developer.mozilla.org/en-US/docs/Web/API/FormData/append + + var vars = param.split('&'), + i = 0, + pair, + key, + value, + formData = new FormData(); + for (i = 0; i < vars.length; i++) { + pair = vars[i].split('='); + key = decodeURIComponent(pair[0]); + value = decodeURIComponent(pair[1]); + formData.append(key, value); + } + // We'll also inform it that we are unloading, potentially useful in the future. + formData.append('unloading', '1'); + + // The results is true or false, we don't get the response from the server. Make it look like it was a success. + navigator.sendBeacon(url, formData); + // This is what a success looks like when it comes back from the server. + return "true\n0"; + } + // httpReq.open (Method("get","post"), URL(string), Asyncronous(true,false)) //popupwin(url+"\n"+param); httpReq.open("POST", url,false); @@ -60,3 +88,24 @@ function popupwin(content) { op.document.write(content); op.document.close(); } + +/** + * We wire up a small marker for the unload events triggered when the user is navigating away or closing the tab. + * This is done because Chrome does not allow synchronous XHR requests on the following unload events: + * - beforeunload + * - unload + * - pagehide + * - visibilitychange + */ +(function() { + // Set up a global var. Sorry about this, old code ... old ways. + window.mod_scorm_is_window_closing = false; + var toggle = function() { + window.mod_scorm_is_window_closing = true; + }; + // Listen to the four events known to represent an unload operation. + window.addEventListener('beforeunload', toggle); + window.addEventListener('unload', toggle); + window.addEventListener('pagehide', toggle); + window.addEventListener('visibilitychange', toggle); +})(); \ No newline at end of file diff --git a/mod/scorm/tests/behat/save_progress_on_unload.feature b/mod/scorm/tests/behat/save_progress_on_unload.feature new file mode 100644 index 00000000000..054d2777b0d --- /dev/null +++ b/mod/scorm/tests/behat/save_progress_on_unload.feature @@ -0,0 +1,51 @@ +@mod @mod_scorm @_file_upload @_switch_iframe @_alert +Feature: Confirm progress gets saved on unload events + In order to let students access a scorm package + As a teacher + I need to add scorm activity to a course + Background: + Given the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | Teacher | 1 | teacher1@example.com | + | student1 | Student | 1 | student1@example.com | + And the following "courses" exist: + | fullname | shortname | category | + | Course 1 | C1 | 0 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | + + @javascript + Scenario: Test progress gets saved correctly when the user navigates away from the scorm activity + Given I log in as "teacher1" + And I am on "Course 1" course homepage with editing mode on + And I add a "SCORM package" to section "1" + And I set the following fields to these values: + | Name | Runtime Basic Calls SCORM 2004 3rd Edition package | + | Description | Description | + And I upload "mod/scorm/tests/packages/RuntimeBasicCalls_SCORM20043rdEdition.zip" file to "Package file" filemanager + And I click on "Save and display" "button" + And I should see "Runtime Basic Calls SCORM 2004 3rd Edition package" + And I log out + When I log in as "student1" + And I am on "Course 1" course homepage + And I follow "Runtime Basic Calls SCORM 2004 3rd Edition package" + Then I should see "Normal" + And I press "Enter" + And I switch to "scorm_object" iframe + And I press "Next" + And I press "Next" + And I switch to "contentFrame" iframe + And I should see "Scoring" + And I switch to the main frame + And I follow "C1" + And I follow "Runtime Basic Calls SCORM 2004 3rd Edition package" + And I should see "Normal" + And I click on "Enter" "button" confirming the dialogue + And I switch to "scorm_object" iframe + And I switch to "contentFrame" iframe + And I should see "Scoring" + And I switch to the main frame + # Go away from the scorm to stop background requests + And I am on homepage \ No newline at end of file