Merge branch 'MDL-68572-master' of https://github.com/catalyst/moodle

mod/quiz/accessrule/seb/classes/quiz_settings.php

@@ -471,6 +471,7 @@ private function process_seb_template() {
         $template = template::get_record(['id' => $this->get('templateid')]);
         $this->plist = new property_list($template->get('content'));
 
+        $this->process_bool_setting('allowuserquitseb');
         $this->process_quit_password_settings();
         $this->process_quit_url_from_template_or_config();
         $this->process_required_enforced_settings();
@@ -492,7 +493,6 @@ private function process_seb_upload_config() {
             $this->plist = new property_list($file->get_content());
         }
 
-        $this->process_quit_password_settings();
         $this->process_quit_url_from_template_or_config();
         $this->process_required_enforced_settings();
 
@@ -525,12 +525,27 @@ private function process_bool_settings() {
         $map = $this->get_bool_seb_setting_map();
         foreach ($settings as $setting => $value) {
             if (isset($map[$setting])) {
-                $enabled = $value == 1 ? true : false;
-                $this->plist->add_element_to_root($map[$setting], new CFBoolean($enabled));
+                $this->process_bool_setting($setting);
             }
         }
     }
 
+    /**
+     * Process provided single bool setting.
+     *
+     * @param string $name Setting name matching one from self::get_bool_seb_setting_map.
+     */
+    private function process_bool_setting(string $name) {
+        $map = $this->get_bool_seb_setting_map();
+
+        if (!isset($map[$name])) {
+            throw new \coding_exception('Provided setting name can not be found in known bool settings');
+        }
+
+        $enabled = $this->raw_get($name) == 1 ? true : false;
+        $this->plist->set_or_update_value($map[$name], new CFBoolean($enabled));
+    }
+
     /**
      * Turn hashed quit password and quit link into PList strings and add to config PList.
      */
@@ -540,6 +555,8 @@ private function process_quit_password_settings() {
             // Hash quit password.
             $hashedpassword = hash('SHA256', $settings->quitpassword);
             $this->plist->add_element_to_root('hashedQuitPassword', new CFString($hashedpassword));
+        } else if (!is_null($this->plist->get_element_value('hashedQuitPassword'))) {
+            $this->plist->delete_element('hashedQuitPassword');
         }
     }
 

mod/quiz/accessrule/seb/classes/settings_provider.php

@@ -902,9 +902,6 @@ public static function get_seb_settings_map() : array {
             self::USE_SEB_UPLOAD_CONFIG => [
                 'filemanager_sebconfigfile' => [],
                 'seb_showsebdownloadlink' => [],
-                'seb_allowuserquitseb' => [
-                    'seb_quitpassword' => [],
-                ],
                 'seb_allowedbrowserexamkeys' => [],
             ],
             self::USE_SEB_CLIENT_CONFIG => [
@@ -976,16 +973,18 @@ public static function get_quiz_hideifs() : array {
             }
         }
 
-        // Specific case for "Enable quitting of SEB". It should available for Manual, Template and Uploaded config.
+        // Specific case for "Enable quitting of SEB". It should available for Manual and Template.
         $hideifs['seb_allowuserquitseb'] = [
             new hideif_rule('seb_allowuserquitseb', 'seb_requiresafeexambrowser', 'eq', self::USE_SEB_NO),
             new hideif_rule('seb_allowuserquitseb', 'seb_requiresafeexambrowser', 'eq', self::USE_SEB_CLIENT_CONFIG),
+            new hideif_rule('seb_allowuserquitseb', 'seb_requiresafeexambrowser', 'eq', self::USE_SEB_UPLOAD_CONFIG),
         ];
 
-        // Specific case for "Quit password". It should be available for Manual, Template and Uploaded config. As it's parent.
+        // Specific case for "Quit password". It should be available for Manual and Template. As it's parent.
         $hideifs['seb_quitpassword'] = [
             new hideif_rule('seb_quitpassword', 'seb_requiresafeexambrowser', 'eq', self::USE_SEB_NO),
             new hideif_rule('seb_quitpassword', 'seb_requiresafeexambrowser', 'eq', self::USE_SEB_CLIENT_CONFIG),
+            new hideif_rule('seb_quitpassword', 'seb_requiresafeexambrowser', 'eq', self::USE_SEB_UPLOAD_CONFIG),
             new hideif_rule('seb_quitpassword', 'seb_allowuserquitseb', 'eq', 0),
         ];
 

mod/quiz/accessrule/seb/tests/behat/edit_form.feature

@@ -103,8 +103,8 @@ Feature: Safe Exam Browser settings in quiz edit form
     And I set the field "Require the use of Safe Exam Browser" to "Yes – Upload my own config"
     Then I should see "Upload Safe Exam Browser config file"
     Then I should see "Show Safe Exam Browser download button"
-    Then I should see "Enable quitting of SEB"
-    Then I should see "Quit password"
+    Then I should not see "Enable quitting of SEB"
+    Then I should not see "Quit password"
     Then I should see "Allowed Browser Exam Keys"
     Then I should not see "Show Exit Safe Exam Browser button, configured with this quit link"
     Then I should not see "Ask user to confirm quitting"
@@ -125,8 +125,6 @@ Feature: Safe Exam Browser settings in quiz edit form
     Then I should not see "Regex blocked"
     Then I should not see "Safe Exam Browser config template"
     Then I should not see "Template 1"
-    And I set the field "Enable quitting of SEB" to "No"
-    Then I should not see "Quit password"
 
   Scenario: SEB settings if using Use an existing template
     Given the following "quizaccess_seb > seb templates" exist:

mod/quiz/accessrule/seb/tests/phpunit/quiz_settings_test.php

@@ -237,49 +237,176 @@ public function test_no_config_file_uploaded() {
     }
 
     /**
-     * Test using USE_SEB_TEMPLATE and have it override defaults.
+     * A helper function to build a config file.
+     *
+     * @param mixed $allowuserquitseb Required allowQuit setting.
+     * @param mixed $quitpassword Required hashedQuitPassword setting.
+     *
+     * @return string
      */
-    public function test_using_seb_template_override_settings() {
-        $template = $this->create_template();
+    protected function get_config_xml($allowuserquitseb = null, $quitpassword = null) {
+        $xml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+            . "<!DOCTYPE plist PUBLIC \"-//Apple//DTD PLIST 1.0//EN\" \"http://www.apple.com/DTDs/PropertyList-1.0.dtd\">\n"
+            . "<plist version=\"1.0\"><dict><key>allowWlan</key><false/><key>startURL</key>"
+            . "<string>https://safeexambrowser.org/start</string>"
+            . "<key>sendBrowserExamKey</key><true/>";
+
+        if (!is_null($allowuserquitseb)) {
+            $allowuserquitseb = empty($allowuserquitseb) ? 'false' : 'true';
+            $xml .= "<key>allowQuit</key><{$allowuserquitseb}/>";
+        }
+
+        if (!is_null($quitpassword)) {
+            $xml .= "<key>hashedQuitPassword</key><string>{$quitpassword}</string>";
+        }
+
+        $xml .= "</dict></plist>\n";
+
+        return $xml;
+    }
+
+    /**
+     * Test using USE_SEB_TEMPLATE and have it override settings from the template when they are set.
+     */
+    public function test_using_seb_template_override_settings_when_they_set_in_template() {
+        $xml = $this->get_config_xml(true, 'password');
+        $template = $this->create_template($xml);
+
         $this->assertContains("<key>startURL</key><string>https://safeexambrowser.org/start</string>", $template->get('content'));
         $this->assertContains("<key>allowQuit</key><true/>", $template->get('content'));
+        $this->assertContains("<key>hashedQuitPassword</key><string>password</string>", $template->get('content'));
 
         $quizsettings = quiz_settings::get_record(['quizid' => $this->quiz->id]);
         $quizsettings->set('requiresafeexambrowser', settings_provider::USE_SEB_TEMPLATE);
         $quizsettings->set('templateid', $template->get('id'));
-        $quizsettings->set('allowuserquitseb', 0);
-        $quizsettings->set('quitpassword', '123');
+        $quizsettings->set('allowuserquitseb', 1);
         $quizsettings->save();
+
         $this->assertContains(
             "<key>startURL</key><string>https://www.example.com/moodle/mod/quiz/view.php?id={$this->quiz->cmid}</string>",
             $quizsettings->get_config()
         );
+
+        $this->assertContains("<key>allowQuit</key><true/>", $quizsettings->get_config());
+        $this->assertNotContains("hashedQuitPassword", $quizsettings->get_config());
+
+        $quizsettings->set('quitpassword', 'new password');
+        $quizsettings->save();
+        $hashedpassword = hash('SHA256', 'new password');
         $this->assertContains("<key>allowQuit</key><true/>", $quizsettings->get_config());
-        $hashedpassword = hash('SHA256', '123');
-        $this->assertNotContains("<key>hashedQuitPassword</key><string>123</string>", $quizsettings->get_config());
+        $this->assertNotContains("<key>hashedQuitPassword</key><string>password</string>", $quizsettings->get_config());
         $this->assertContains("<key>hashedQuitPassword</key><string>{$hashedpassword}</string>", $quizsettings->get_config());
+
+        $quizsettings->set('allowuserquitseb', 0);
+        $quizsettings->set('quitpassword', '');
+        $quizsettings->save();
+        $this->assertContains("<key>allowQuit</key><false/>", $quizsettings->get_config());
+        $this->assertNotContains("hashedQuitPassword", $quizsettings->get_config());
     }
 
     /**
-     * Test using USE_SEB_UPLOAD_CONFIG and overriding the password.
+     * Test using USE_SEB_TEMPLATE and have it override settings from the template when they are not set.
      */
-    public function test_using_own_config_and_overriding_password() {
-        $url = new moodle_url("/mod/quiz/view.php", ['id' => $this->quiz->cmid]);
-        $xml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
-            . "<!DOCTYPE plist PUBLIC \"-//Apple//DTD PLIST 1.0//EN\" \"http://www.apple.com/DTDs/PropertyList-1.0.dtd\">\n"
-            . "<plist version=\"1.0\"><dict><key>hashedQuitPassword</key><string>hashedpassword</string>"
-            . "<key>allowWlan</key><false/><key>startURL</key><string>$url</string>"
-            . "<key>sendBrowserExamKey</key><true/></dict></plist>\n";
-        $itemid = $this->create_module_test_file($xml, $this->quiz->cmid);
+    public function test_using_seb_template_override_settings_when_not_set_in_template() {
+        $xml = $this->get_config_xml();
+        $template = $this->create_template($xml);
+
+        $this->assertContains("<key>startURL</key><string>https://safeexambrowser.org/start</string>", $template->get('content'));
+        $this->assertNotContains("<key>allowQuit</key><true/>", $template->get('content'));
+        $this->assertNotContains("<key>hashedQuitPassword</key><string>password</string>", $template->get('content'));
+
+        $quizsettings = quiz_settings::get_record(['quizid' => $this->quiz->id]);
+        $quizsettings->set('requiresafeexambrowser', settings_provider::USE_SEB_TEMPLATE);
+        $quizsettings->set('templateid', $template->get('id'));
+        $quizsettings->set('allowuserquitseb', 1);
+        $quizsettings->save();
+
+        $this->assertContains("<key>allowQuit</key><true/>", $quizsettings->get_config());
+        $this->assertNotContains("hashedQuitPassword", $quizsettings->get_config());
+
+        $quizsettings->set('quitpassword', 'new password');
+        $quizsettings->save();
+        $hashedpassword = hash('SHA256', 'new password');
+        $this->assertContains("<key>allowQuit</key><true/>", $quizsettings->get_config());
+        $this->assertContains("<key>hashedQuitPassword</key><string>{$hashedpassword}</string>", $quizsettings->get_config());
+
+        $quizsettings->set('allowuserquitseb', 0);
+        $quizsettings->set('quitpassword', '');
+        $quizsettings->save();
+        $this->assertContains("<key>allowQuit</key><false/>", $quizsettings->get_config());
+        $this->assertNotContains("hashedQuitPassword", $quizsettings->get_config());
+    }
+
+    /**
+     * Test using USE_SEB_UPLOAD_CONFIG and use settings from the file if they are set.
+     */
+    public function test_using_own_config_settings_are_not_overridden_if_set() {
+        $xml = $this->get_config_xml(true, 'password');
+        $this->create_module_test_file($xml, $this->quiz->cmid);
+
         $quizsettings = quiz_settings::get_record(['quizid' => $this->quiz->id]);
         $quizsettings->set('requiresafeexambrowser', settings_provider::USE_SEB_UPLOAD_CONFIG);
-        $quizsettings->set('quitpassword', '123');
+        $quizsettings->set('allowuserquitseb', 0);
+        $quizsettings->set('quitpassword', '');
+        $quizsettings->save();
+
+        $this->assertContains(
+            "<key>startURL</key><string>https://www.example.com/moodle/mod/quiz/view.php?id={$this->quiz->cmid}</string>",
+            $quizsettings->get_config()
+        );
+
+        $this->assertContains("<key>allowQuit</key><true/>", $quizsettings->get_config());
+        $this->assertContains("<key>hashedQuitPassword</key><string>password</string>", $quizsettings->get_config());
+
+        $quizsettings->set('quitpassword', 'new password');
+        $quizsettings->save();
+        $hashedpassword = hash('SHA256', 'new password');
+
+        $this->assertNotContains("<key>hashedQuitPassword</key><string>{$hashedpassword}</string>", $quizsettings->get_config());
+        $this->assertContains("<key>allowQuit</key><true/>", $quizsettings->get_config());
+        $this->assertContains("<key>hashedQuitPassword</key><string>password</string>", $quizsettings->get_config());
+
+        $quizsettings->set('allowuserquitseb', 0);
+        $quizsettings->set('quitpassword', '');
+        $quizsettings->save();
+
+        $this->assertContains("<key>allowQuit</key><true/>", $quizsettings->get_config());
+        $this->assertContains("<key>hashedQuitPassword</key><string>password</string>", $quizsettings->get_config());
+    }
+
+    /**
+     * Test using USE_SEB_UPLOAD_CONFIG and use settings from the file if they are not set.
+     */
+    public function test_using_own_config_settings_are_not_overridden_if_not_set() {
+        $xml = $this->get_config_xml();
+        $this->create_module_test_file($xml, $this->quiz->cmid);
+
+        $quizsettings = quiz_settings::get_record(['quizid' => $this->quiz->id]);
+        $quizsettings->set('requiresafeexambrowser', settings_provider::USE_SEB_UPLOAD_CONFIG);
+        $quizsettings->set('allowuserquitseb', 1);
+        $quizsettings->set('quitpassword', '');
+        $quizsettings->save();
+
+        $this->assertContains(
+            "<key>startURL</key><string>https://www.example.com/moodle/mod/quiz/view.php?id={$this->quiz->cmid}</string>",
+            $quizsettings->get_config()
+        );
+
+        $this->assertNotContains("allowQuit", $quizsettings->get_config());
+        $this->assertNotContains("hashedQuitPassword", $quizsettings->get_config());
+
+        $quizsettings->set('quitpassword', 'new password');
+        $quizsettings->save();
+
+        $this->assertNotContains("allowQuit", $quizsettings->get_config());
+        $this->assertNotContains("hashedQuitPassword", $quizsettings->get_config());
+
+        $quizsettings->set('allowuserquitseb', 0);
+        $quizsettings->set('quitpassword', '');
         $quizsettings->save();
-        $config = $quizsettings->get_config();
 
-        $hashedpassword = hash('SHA256', '123');
-        $this->assertNotContains("<key>hashedQuitPassword</key><string>hashedpassword</string>", $config);
-        $this->assertContains("<key>hashedQuitPassword</key><string>{$hashedpassword}</string>", $config);
+        $this->assertNotContains("allowQuit", $quizsettings->get_config());
+        $this->assertNotContains("hashedQuitPassword", $quizsettings->get_config());
     }
 
     /**

mod/quiz/accessrule/seb/tests/phpunit/settings_provider_test.php

@@ -286,7 +286,7 @@ public function test_hideifs() {
         );
 
         $this->assertArrayHasKey('seb_allowuserquitseb', $settinghideifs);
-        $this->assertCount(2, $settinghideifs['seb_allowuserquitseb']);
+        $this->assertCount(3, $settinghideifs['seb_allowuserquitseb']);
         $this->assert_hide_if(
             $settinghideifs['seb_allowuserquitseb'][0],
             'seb_allowuserquitseb',
@@ -301,9 +301,16 @@ public function test_hideifs() {
             'eq',
             settings_provider::USE_SEB_CLIENT_CONFIG
         );
+        $this->assert_hide_if(
+            $settinghideifs['seb_allowuserquitseb'][2],
+            'seb_allowuserquitseb',
+            'seb_requiresafeexambrowser',
+            'eq',
+            settings_provider::USE_SEB_UPLOAD_CONFIG
+        );
 
         $this->assertArrayHasKey('seb_quitpassword', $settinghideifs);
-        $this->assertCount(3, $settinghideifs['seb_quitpassword']);
+        $this->assertCount(4, $settinghideifs['seb_quitpassword']);
         $this->assert_hide_if(
             $settinghideifs['seb_quitpassword'][0],
             'seb_quitpassword',
@@ -321,6 +328,13 @@ public function test_hideifs() {
         $this->assert_hide_if(
             $settinghideifs['seb_quitpassword'][2],
             'seb_quitpassword',
+            'seb_requiresafeexambrowser',
+            'eq',
+            settings_provider::USE_SEB_UPLOAD_CONFIG
+        );
+        $this->assert_hide_if(
+            $settinghideifs['seb_quitpassword'][3],
+            'seb_quitpassword',
             'seb_allowuserquitseb',
             'eq',
             0
@@ -1267,8 +1281,7 @@ public function test_filter_plugin_settings_for_no_seb() {
      * Test filter_plugin_settings method for using uploaded config.
      */
     public function test_filter_plugin_settings_for_uploaded_config() {
-        $notnulls = ['requiresafeexambrowser', 'showsebdownloadlink', 'allowuserquitseb',
-            'quitpassword', 'allowedbrowserexamkeys'];
+        $notnulls = ['requiresafeexambrowser', 'showsebdownloadlink', 'allowedbrowserexamkeys'];
         $this->assert_filter_plugin_settings(settings_provider::USE_SEB_UPLOAD_CONFIG, $notnulls);
     }
 
@@ -1353,9 +1366,6 @@ public function test_get_seb_settings_map() {
             settings_provider::USE_SEB_UPLOAD_CONFIG => [
                 'filemanager_sebconfigfile' => [],
                 'seb_showsebdownloadlink' => [],
-                'seb_allowuserquitseb' => [
-                    'seb_quitpassword' => [],
-                ],
                 'seb_allowedbrowserexamkeys' => [],
             ],
             settings_provider::USE_SEB_CLIENT_CONFIG => [