Merge branch 'MDL-67175-master' of git://github.com/andrewnicols/moodle

lib/classes/session/manager.php

@@ -292,7 +292,27 @@ protected static function prepare_cookies() {
 
         // 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.
@@ -455,6 +475,8 @@ protected static function initialise_user_session($newsid) {
         if ($timedout) {
             $_SESSION['SESSION']->has_timed_out = true;
         }
+
+        self::append_samesite_cookie_attribute();
     }
 
     /**
@@ -522,6 +544,61 @@ public static function login_user(\stdClass $user) {
 
         // 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(): bool {
+        // 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);
+        }
     }
 
     /**
@@ -558,6 +635,8 @@ public static function terminate_current() {
         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();
     }
 
     /**

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];

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:' + "<?php p($CFG->wwwroot) ?>/mod/scorm/datamodel.php" + "id=<?php p($id) ?>&a=<?php p($a) ?>&sesskey=<?php echo 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];

mod/scorm/datamodels/scorm_13.js

@@ -1216,7 +1216,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 != '')) {

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) {

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);
+})();

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