Merge branch 'MDL-75372-url_blocked' of https://github.com/catalyst/moodle

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

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

lib/classes/event/url_blocked.php

@@ -0,0 +1,116 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+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
+        );
+    }
+}

lib/classes/local/guzzle/check_request.php

@@ -84,6 +84,8 @@ class check_request {
      * @return PromiseInterface
      */
     public function __invoke(RequestInterface $request, array $options): PromiseInterface {
+        global $USER;
+
         $fn = $this->nexthandler;
         $settings = $this->settings;
 
@@ -99,7 +101,13 @@ class check_request {
         }
 
         if ($this->securityhelper->url_is_blocked((string) $request->getUri())) {
-            throw new RequestException($this->securityhelper->get_blocked_url_string(), $request);
+            $msg = $this->securityhelper->get_blocked_url_string();
+            debugging(
+                sprintf('Blocked %s [user %d]', $msg, $USER->id),
+                DEBUG_NONE
+            );
+
+            throw new RequestException($msg, $request);
         }
 
         return $fn($request, $options);

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

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

lib/tests/http_client_test.php

@@ -207,17 +207,22 @@ class http_client_test extends \advanced_testcase {
         $mockhelper = $this->getMockBuilder('\core\files\curl_security_helper')->getMock();
 
         // Make the mock return a different string.
-        $mockhelper->expects($this->any())->method('get_blocked_url_string')->will($this->returnValue('You shall not pass'));
+        $blocked = "http://blocked.com";
+        $mockhelper->expects($this->any())->method('get_blocked_url_string')->will($this->returnValue($blocked));
 
         // And make the mock security helper block all URLs. This helper instance doesn't care about config.
         $mockhelper->expects($this->any())->method('url_is_blocked')->will($this->returnValue(true));
 
-        $mock = new MockHandler([new Response(200, [], 'You shall not pass')]);
-        $client = new \core\http_client(['mock' => $mock, 'securityhelper' => $mockhelper]);
-        $this->expectException(\GuzzleHttp\Exception\RequestException::class);
-        $response = $client->request('GET', $testhtml);
+        $client = new \core\http_client(['securityhelper' => $mockhelper]);
+
+        $this->resetDebugging();
+        try {
+            $client->request('GET', $testhtml);
+            $this->fail("Blocked Request should have thrown an exception");
+        } catch (\GuzzleHttp\Exception\RequestException $e) {
+            $this->assertDebuggingCalled("Blocked $blocked [user 0]", DEBUG_NONE);
+        }
 
-        $this->assertSame('You shall not pass', $response->getBody()->getContents());
     }
 
     /**