From 5813741877ace69c8db035c9e9ccf3ba75fa8447 Mon Sep 17 00:00:00 2001 From: Srdjan Date: Wed, 23 Nov 2022 16:27:25 +1000 Subject: [PATCH] MDL-75372 core: url_blocked event Triggered with check_securityhelper_blocklist(). The event is also emitting a debugging() message. --- lang/en/moodle.php | 1 + lib/classes/event/base.php | 2 +- lib/classes/event/url_blocked.php | 116 ++++++++++++++++++++++++++++++ lib/filelib.php | 19 +++++ lib/tests/filelib_test.php | 25 +++++++ 5 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 lib/classes/event/url_blocked.php diff --git a/lang/en/moodle.php b/lang/en/moodle.php index f737937547d..a9bd5d3338d 100644 --- a/lang/en/moodle.php +++ b/lang/en/moodle.php @@ -843,6 +843,7 @@ $string['eventrecentactivityviewed'] = 'Recent activity viewed'; $string['eventsearchindexed'] = 'Search data indexed'; $string['eventsearchresultsviewed'] = 'Search results viewed'; $string['eventunknownlogged'] = 'Unknown event'; +$string['eventurlblocked'] = 'The URL was blocked'; $string['eventusercreated'] = 'User created'; $string['eventuserdeleted'] = 'User deleted'; $string['eventuserfeedbackgiven'] = 'Feedback link clicked'; diff --git a/lib/classes/event/base.php b/lib/classes/event/base.php index 904c9f88a72..9a008d6ce72 100644 --- a/lib/classes/event/base.php +++ b/lib/classes/event/base.php @@ -712,7 +712,7 @@ abstract class base implements \IteratorAggregate { * * @throws \coding_exception */ - protected final function validate_before_trigger() { + protected function validate_before_trigger() { global $DB, $CFG; if (empty($this->data['crud'])) { diff --git a/lib/classes/event/url_blocked.php b/lib/classes/event/url_blocked.php new file mode 100644 index 00000000000..bc8881391d1 --- /dev/null +++ b/lib/classes/event/url_blocked.php @@ -0,0 +1,116 @@ +. + +namespace core\event; + +/** + * URL blocked event class. + * + * @property-read array $other { + * Extra information about event. + * + * - string url: blocked url + * - string reason: reason for blocking + * - bool redirect: blocked url was a redirect + * } + * + * @package core + * @copyright 2022 Catalyst IT + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class url_blocked extends base { + + /** + * Returns description of what happened. + * + * @return string + */ + public function get_description() { + return sprintf( + 'Blocked %s%s: %s', + $this->other['url'], + $this->other['redirect'] ? ' (redirect)' : '', + $this->other['reason'] + ); + } + + /** + * Return localised event name. + * + * @return string + */ + public static function get_name() { + return get_string('eventurlblocked', 'core'); + } + + /** + * Init method. + * + * @return void + */ + protected function init() { + global $USER; + + $this->data['crud'] = 'c'; + $this->data['edulevel'] = self::LEVEL_OTHER; + $this->context = empty($USER->id) ? \context_system::instance() : \context_user::instance($USER->id); + } + + /** + * Custom validation. + * + * It is recommended to set the properties: + * - $other['tokenid'] + * - $other['username'] + * + * However they are not mandatory as they are not always known. + * + * Please note that the token CANNOT be specified, it is considered + * as a password and should never be displayed. + * + * @throws \coding_exception + * @return void + */ + protected function validate_data() { + parent::validate_data(); + if (!isset($this->other['url'])) { + throw new \coding_exception("The 'url' value must be set in other."); + } + } + + /** + * Used when restoring course logs. + * + */ + public static function get_other_mapping() { + } + + /** + * Validate all properties right before triggering the event. + * + * Emits debugging. + * + * @throws \coding_exception + */ + protected function validate_before_trigger() { + parent::validate_before_trigger(); + + debugging( + sprintf('%s [user %d]', $this->get_description(), $this->userid), + DEBUG_NONE + ); + } +} diff --git a/lib/filelib.php b/lib/filelib.php index 59bf8066fda..48769d19cce 100644 --- a/lib/filelib.php +++ b/lib/filelib.php @@ -3779,6 +3779,7 @@ class curl { $urlisblocked = $this->check_securityhelper_blocklist($url); if (!is_null($urlisblocked)) { + $this->trigger_url_blocked_event($url, $urlisblocked); return $urlisblocked; } @@ -3879,6 +3880,7 @@ class curl { if (!is_null($urlisblocked)) { $this->reset_request_state_vars(); curl_close($curl); + $this->trigger_url_blocked_event($redirecturl, $urlisblocked, true); return $urlisblocked; } @@ -3941,6 +3943,23 @@ class curl { } /** + * Trigger url_blocked event + * + * @param string $url The URL to request + * @param string $reason Reason for blocking + * @param bool $redirect true if it was a redirect + */ + private function trigger_url_blocked_event($url, $reason, $redirect = false): void { + $params = [ + 'url' => $url, + 'reason' => $reason, + 'redirect' => $redirect, + ]; + $event = core\event\url_blocked::create(['other' => $params]); + $event->trigger(); + } + + /** * HTTP HEAD method * * @see request() diff --git a/lib/tests/filelib_test.php b/lib/tests/filelib_test.php index 2cd8159526c..02c175edcf7 100644 --- a/lib/tests/filelib_test.php +++ b/lib/tests/filelib_test.php @@ -247,8 +247,12 @@ class filelib_test extends \advanced_testcase { * Test a curl basic request with security enabled. */ public function test_curl_basics_with_security_helper() { + global $USER; + $this->resetAfterTest(); + $sink = $this->redirectEvents(); + // Test a request with a basic hostname filter applied. $testhtml = $this->getExternalTestFileUrl('/test.html'); $url = new \moodle_url($testhtml); @@ -261,6 +265,18 @@ class filelib_test extends \advanced_testcase { $expected = $curl->get_security()->get_blocked_url_string(); $this->assertSame($expected, $contents); $this->assertSame(0, $curl->get_errno()); + $this->assertDebuggingCalled( + "Blocked $testhtml: The URL is blocked. [user {$USER->id}]", DEBUG_NONE); + + $events = $sink->get_events(); + $this->assertCount(1, $events); + $event = reset($events); + + $this->assertEquals('\core\event\url_blocked', $event->eventname); + $this->assertEquals("Blocked $testhtml: $expected", $event->get_description()); + $this->assertEquals(\context_system::instance(), $event->get_context()); + $this->assertEquals($testhtml, $event->other['url']); + $this->assertEventContextNotUsed($event); // Now, create a curl using the 'ignoresecurity' override. // We expect this request to pass, despite the admin setting having been set earlier. @@ -269,6 +285,9 @@ class filelib_test extends \advanced_testcase { $this->assertSame('47250a973d1b88d9445f94db4ef2c97a', md5($contents)); $this->assertSame(0, $curl->get_errno()); + $events = $sink->get_events(); + $this->assertCount(1, $events); + // Now, try injecting a mock security helper into curl. This will override the default helper. $mockhelper = $this->getMockBuilder('\core\files\curl_security_helper')->getMock(); @@ -282,6 +301,10 @@ class filelib_test extends \advanced_testcase { $contents = $curl->get($testhtml); $this->assertSame('You shall not pass', $curl->get_security()->get_blocked_url_string()); $this->assertSame($curl->get_security()->get_blocked_url_string(), $contents); + $this->assertDebuggingCalled(); + + $events = $sink->get_events(); + $this->assertCount(2, $events); } public function test_curl_redirects() { @@ -407,12 +430,14 @@ class filelib_test extends \advanced_testcase { $contents = $curl->get("{$testurl}?redir=1&extdest=1"); $this->assertSame($blockedstring, $contents); $this->assertSame(0, $curl->get_errno()); + $this->assertDebuggingCalled(); // Redirecting to the blocked host after multiple successful redirects should also fail. $curl = new \curl(); $contents = $curl->get("{$testurl}?redir=3&extdest=1"); $this->assertSame($blockedstring, $contents); $this->assertSame(0, $curl->get_errno()); + $this->assertDebuggingCalled(); } public function test_curl_relative_redirects() {