From 93532e7871d92b66c6b1d1de036eff2e888c45e4 Mon Sep 17 00:00:00 2001 From: Dmitrii Metelkin Date: Wed, 6 May 2020 15:44:49 +1000 Subject: [PATCH] MDL-68572 quizaccess: use uploaded file as is for SEB settings --- .../accessrule/seb/classes/quiz_settings.php | 23 ++- .../seb/classes/settings_provider.php | 9 +- .../seb/tests/behat/edit_form.feature | 6 +- .../seb/tests/phpunit/quiz_settings_test.php | 169 +++++++++++++++--- .../tests/phpunit/settings_provider_test.php | 24 ++- 5 files changed, 191 insertions(+), 40 deletions(-) diff --git a/mod/quiz/accessrule/seb/classes/quiz_settings.php b/mod/quiz/accessrule/seb/classes/quiz_settings.php index 9511efefd6c..c621a5114f9 100644 --- a/mod/quiz/accessrule/seb/classes/quiz_settings.php +++ b/mod/quiz/accessrule/seb/classes/quiz_settings.php @@ -471,6 +471,7 @@ class quiz_settings extends persistent { $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 @@ class quiz_settings extends persistent { $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 @@ class quiz_settings extends persistent { $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 @@ class quiz_settings extends persistent { // 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'); } } diff --git a/mod/quiz/accessrule/seb/classes/settings_provider.php b/mod/quiz/accessrule/seb/classes/settings_provider.php index 86ef59306d0..40805bfab1a 100644 --- a/mod/quiz/accessrule/seb/classes/settings_provider.php +++ b/mod/quiz/accessrule/seb/classes/settings_provider.php @@ -902,9 +902,6 @@ class settings_provider { self::USE_SEB_UPLOAD_CONFIG => [ 'filemanager_sebconfigfile' => [], 'seb_showsebdownloadlink' => [], - 'seb_allowuserquitseb' => [ - 'seb_quitpassword' => [], - ], 'seb_allowedbrowserexamkeys' => [], ], self::USE_SEB_CLIENT_CONFIG => [ @@ -976,16 +973,18 @@ class settings_provider { } } - // 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), ]; diff --git a/mod/quiz/accessrule/seb/tests/behat/edit_form.feature b/mod/quiz/accessrule/seb/tests/behat/edit_form.feature index ea9af1d8d64..90bbbc62329 100644 --- a/mod/quiz/accessrule/seb/tests/behat/edit_form.feature +++ b/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: diff --git a/mod/quiz/accessrule/seb/tests/phpunit/quiz_settings_test.php b/mod/quiz/accessrule/seb/tests/phpunit/quiz_settings_test.php index c65caab7a07..e44bf5ea65f 100644 --- a/mod/quiz/accessrule/seb/tests/phpunit/quiz_settings_test.php +++ b/mod/quiz/accessrule/seb/tests/phpunit/quiz_settings_test.php @@ -237,49 +237,176 @@ class quizaccess_seb_quiz_settings_testcase extends quizaccess_seb_testcase { } /** - * 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 = "\n" + . "\n" + . "allowWlanstartURL" + . "https://safeexambrowser.org/start" + . "sendBrowserExamKey"; + + if (!is_null($allowuserquitseb)) { + $allowuserquitseb = empty($allowuserquitseb) ? 'false' : 'true'; + $xml .= "allowQuit<{$allowuserquitseb}/>"; + } + + if (!is_null($quitpassword)) { + $xml .= "hashedQuitPassword{$quitpassword}"; + } + + $xml .= "\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("startURLhttps://safeexambrowser.org/start", $template->get('content')); $this->assertContains("allowQuit", $template->get('content')); + $this->assertContains("hashedQuitPasswordpassword", $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( "startURLhttps://www.example.com/moodle/mod/quiz/view.php?id={$this->quiz->cmid}", $quizsettings->get_config() ); + $this->assertContains("allowQuit", $quizsettings->get_config()); - $hashedpassword = hash('SHA256', '123'); - $this->assertNotContains("hashedQuitPassword123", $quizsettings->get_config()); + $this->assertNotContains("hashedQuitPassword", $quizsettings->get_config()); + + $quizsettings->set('quitpassword', 'new password'); + $quizsettings->save(); + $hashedpassword = hash('SHA256', 'new password'); + $this->assertContains("allowQuit", $quizsettings->get_config()); + $this->assertNotContains("hashedQuitPasswordpassword", $quizsettings->get_config()); $this->assertContains("hashedQuitPassword{$hashedpassword}", $quizsettings->get_config()); + + $quizsettings->set('allowuserquitseb', 0); + $quizsettings->set('quitpassword', ''); + $quizsettings->save(); + $this->assertContains("allowQuit", $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 = "\n" - . "\n" - . "hashedQuitPasswordhashedpassword" - . "allowWlanstartURL$url" - . "sendBrowserExamKey\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("startURLhttps://safeexambrowser.org/start", $template->get('content')); + $this->assertNotContains("allowQuit", $template->get('content')); + $this->assertNotContains("hashedQuitPasswordpassword", $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("allowQuit", $quizsettings->get_config()); + $this->assertNotContains("hashedQuitPassword", $quizsettings->get_config()); + + $quizsettings->set('quitpassword', 'new password'); + $quizsettings->save(); + $hashedpassword = hash('SHA256', 'new password'); + $this->assertContains("allowQuit", $quizsettings->get_config()); + $this->assertContains("hashedQuitPassword{$hashedpassword}", $quizsettings->get_config()); + + $quizsettings->set('allowuserquitseb', 0); + $quizsettings->set('quitpassword', ''); + $quizsettings->save(); + $this->assertContains("allowQuit", $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(); - $config = $quizsettings->get_config(); - $hashedpassword = hash('SHA256', '123'); - $this->assertNotContains("hashedQuitPasswordhashedpassword", $config); - $this->assertContains("hashedQuitPassword{$hashedpassword}", $config); + $this->assertContains( + "startURLhttps://www.example.com/moodle/mod/quiz/view.php?id={$this->quiz->cmid}", + $quizsettings->get_config() + ); + + $this->assertContains("allowQuit", $quizsettings->get_config()); + $this->assertContains("hashedQuitPasswordpassword", $quizsettings->get_config()); + + $quizsettings->set('quitpassword', 'new password'); + $quizsettings->save(); + $hashedpassword = hash('SHA256', 'new password'); + + $this->assertNotContains("hashedQuitPassword{$hashedpassword}", $quizsettings->get_config()); + $this->assertContains("allowQuit", $quizsettings->get_config()); + $this->assertContains("hashedQuitPasswordpassword", $quizsettings->get_config()); + + $quizsettings->set('allowuserquitseb', 0); + $quizsettings->set('quitpassword', ''); + $quizsettings->save(); + + $this->assertContains("allowQuit", $quizsettings->get_config()); + $this->assertContains("hashedQuitPasswordpassword", $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( + "startURLhttps://www.example.com/moodle/mod/quiz/view.php?id={$this->quiz->cmid}", + $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(); + + $this->assertNotContains("allowQuit", $quizsettings->get_config()); + $this->assertNotContains("hashedQuitPassword", $quizsettings->get_config()); } /** diff --git a/mod/quiz/accessrule/seb/tests/phpunit/settings_provider_test.php b/mod/quiz/accessrule/seb/tests/phpunit/settings_provider_test.php index 68253313613..0ffb3788932 100644 --- a/mod/quiz/accessrule/seb/tests/phpunit/settings_provider_test.php +++ b/mod/quiz/accessrule/seb/tests/phpunit/settings_provider_test.php @@ -286,7 +286,7 @@ class quizaccess_seb_settings_provider_testcase extends quizaccess_seb_testcase ); $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 @@ class quizaccess_seb_settings_provider_testcase extends quizaccess_seb_testcase '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 @@ class quizaccess_seb_settings_provider_testcase extends quizaccess_seb_testcase $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 @@ class quizaccess_seb_settings_provider_testcase extends quizaccess_seb_testcase * 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 @@ class quizaccess_seb_settings_provider_testcase extends quizaccess_seb_testcase settings_provider::USE_SEB_UPLOAD_CONFIG => [ 'filemanager_sebconfigfile' => [], 'seb_showsebdownloadlink' => [], - 'seb_allowuserquitseb' => [ - 'seb_quitpassword' => [], - ], 'seb_allowedbrowserexamkeys' => [], ], settings_provider::USE_SEB_CLIENT_CONFIG => [