From 1dda6c86b80335a7dcb7e8299702516db978b891 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Tue, 12 Sep 2023 11:37:36 +0700 Subject: [PATCH] MDL-79041 core: Better handling of the MoodleNet resource URL - Switch use PARAM_TEXT instead of PARAM_URL for resource URL - Added noreferrer to the Go to MoodleNet drafts button, to avoid the risks associated with opening in _blank without removing access to the referrer and opener --- .../external/moodlenet_send_activity.php | 5 +- .../external/moodlenet_send_course.php | 5 +- .../send_activity_modal_footer_view.mustache | 2 +- .../external/moodlenet_send_activity_test.php | 57 +++++++++++++++++++ .../external/moodlenet_send_course_test.php | 57 +++++++++++++++++++ 5 files changed, 123 insertions(+), 3 deletions(-) diff --git a/lib/classes/external/moodlenet_send_activity.php b/lib/classes/external/moodlenet_send_activity.php index edb093eb25f..33320f27570 100644 --- a/lib/classes/external/moodlenet_send_activity.php +++ b/lib/classes/external/moodlenet_send_activity.php @@ -153,7 +153,10 @@ class moodlenet_send_activity extends external_api { public static function execute_returns(): external_single_structure { return new external_single_structure([ 'status' => new external_value(PARAM_BOOL, 'Status: true if success'), - 'resourceurl' => new external_value(PARAM_URL, 'Resource URL from MoodleNet'), + // We used PARAM_TEXT instead of PARAM_URL because the URL return from MoodleNet may contain some characters. + // It does not match with PARAM_URL, but the URL still works. + // Since we just show the response resource URL to the user for them to navigate to MoodleNet, it would be safe. + 'resourceurl' => new external_value(PARAM_TEXT, 'Resource URL from MoodleNet'), 'warnings' => new external_warnings(), ]); } diff --git a/lib/classes/external/moodlenet_send_course.php b/lib/classes/external/moodlenet_send_course.php index 69b3425fc36..d625e98ea6e 100644 --- a/lib/classes/external/moodlenet_send_course.php +++ b/lib/classes/external/moodlenet_send_course.php @@ -212,7 +212,10 @@ class moodlenet_send_course extends external_api { public static function execute_returns(): external_single_structure { return new external_single_structure([ 'status' => new external_value(PARAM_BOOL, 'Status: true if success'), - 'resourceurl' => new external_value(PARAM_URL, 'Resource URL from MoodleNet'), + // We used PARAM_TEXT instead of PARAM_URL because the URL return from MoodleNet may contain some characters. + // It does not match with PARAM_URL, but the URL still works. + // Since we just show the response resource URL to the user for them to navigate to MoodleNet, it would be safe. + 'resourceurl' => new external_value(PARAM_TEXT, 'Resource URL from MoodleNet'), 'warnings' => new external_warnings(), ]); } diff --git a/lib/templates/moodlenet/send_activity_modal_footer_view.mustache b/lib/templates/moodlenet/send_activity_modal_footer_view.mustache index 2c82e124f38..425f5013790 100644 --- a/lib/templates/moodlenet/send_activity_modal_footer_view.mustache +++ b/lib/templates/moodlenet/send_activity_modal_footer_view.mustache @@ -20,7 +20,7 @@ } }}
- + {{#str}} moodlenet:gotomoodlenet, moodle {{/str}}
diff --git a/lib/tests/external/moodlenet_send_activity_test.php b/lib/tests/external/moodlenet_send_activity_test.php index 0fb23c248ce..fdaebf8726d 100644 --- a/lib/tests/external/moodlenet_send_activity_test.php +++ b/lib/tests/external/moodlenet_send_activity_test.php @@ -111,4 +111,61 @@ class moodlenet_send_activity_test extends externallib_advanced_testcase { $this->assertEquals($issuer->get('id'), $result['warnings'][0]['item']); $this->assertEquals(get_string('moodlenet:issuerisnotauthorized', 'moodle'), $result['warnings'][0]['message']); } + + /** + * Test execute_returns() method. + * + * @dataProvider return_resource_url_provider + * @covers ::execute_returns + */ + public function test_moodlenet_send_activity_return_resource_url(bool $state, string $resourceurl) { + $this->resetAfterTest(); + // Create dummy result with the resourceurl. + $result = [ + 'status' => true, + 'resourceurl' => $resourceurl, + 'warnings' => [], + ]; + if (!$state) { + $this->expectException(\invalid_response_exception::class); + } + $result = external_api::clean_returnvalue(moodlenet_send_activity::execute_returns(), $result); + if ($state) { + $this->assertEquals($resourceurl, $result['resourceurl']); + } + } + + /** + * Provider for test_moodlenet_send_activity_return_resource_url(). + * + * @return array Test data. + */ + public function return_resource_url_provider(): array { + return [ + 'Success 1' => [ + true, + 'https://moodlenet.example.com/drafts/view/testactivity_backup.mbz', + ], + 'Success 2' => [ + true, + 'https://moodlenet.example.com/drafts/view/testactivity_backup with spaces.mbz', + ], + 'Success 3' => [ + true, + 'https://moodlenet.example.com/drafts/view/testactivity_backup with " character.mbz', + ], + 'Success 4' => [ + true, + "https://moodlenet.example.com/drafts/view/testactivity_backup with ' character.mbz", + ], + 'Success 5' => [ + true, + 'https://moodlenet.example.com/drafts/view/testactivity_backup with < and > characters.mbz', + ], + 'Fail 1' => [ + false, + 'https://moodlenet.example.com/drafts/view/testactivity_backupwithahtml.mbz', + ], + ]; + } } diff --git a/lib/tests/external/moodlenet_send_course_test.php b/lib/tests/external/moodlenet_send_course_test.php index f9d46edf6b3..85d2c0483bb 100644 --- a/lib/tests/external/moodlenet_send_course_test.php +++ b/lib/tests/external/moodlenet_send_course_test.php @@ -117,4 +117,61 @@ class moodlenet_send_course_test extends externallib_advanced_testcase { $this->assertEquals($issuer->get('id'), $result['warnings'][0]['item']); $this->assertEquals(get_string('moodlenet:issuerisnotauthorized', 'moodle'), $result['warnings'][0]['message']); } + + /** + * Test execute_returns() method. + * + * @dataProvider return_resource_url_provider + * @covers ::execute_returns + */ + public function test_moodlenet_send_course_return_resource_url(bool $state, string $resourceurl) { + $this->resetAfterTest(); + // Create dummy result with the resourceurl. + $result = [ + 'status' => true, + 'resourceurl' => $resourceurl, + 'warnings' => [], + ]; + if (!$state) { + $this->expectException(\invalid_response_exception::class); + } + $result = external_api::clean_returnvalue(moodlenet_send_course::execute_returns(), $result); + if ($state) { + $this->assertEquals($resourceurl, $result['resourceurl']); + } + } + + /** + * Provider for test_moodlenet_send_course_return_resource_url(). + * + * @return array Test data. + */ + public function return_resource_url_provider(): array { + return [ + 'Success 1' => [ + true, + 'https://moodlenet.example.com/drafts/view/testcourse_backup.mbz', + ], + 'Success 2' => [ + true, + 'https://moodlenet.example.com/drafts/view/testcourse_backup with spaces.mbz', + ], + 'Success 3' => [ + true, + 'https://moodlenet.example.com/drafts/view/testcourse_backup with " character.mbz', + ], + 'Success 4' => [ + true, + "https://moodlenet.example.com/drafts/view/testcourse_backup with ' character.mbz", + ], + 'Success 5' => [ + true, + 'https://moodlenet.example.com/drafts/view/testcourse_backup with < and > characters.mbz', + ], + 'Fail 1' => [ + false, + 'https://moodlenet.example.com/drafts/view/testcourse_backupwithahtml.mbz', + ], + ]; + } }