From b0539cb6a80ee133f81ada01bd5aa1058adf3530 Mon Sep 17 00:00:00 2001 From: David Woloszyn Date: Fri, 7 Feb 2025 12:53:42 +1100 Subject: [PATCH] MDL-84351 tool_mfa: Check URL against allowed components for redirect --- admin/tool/mfa/classes/manager.php | 49 +++++++++++++++++++++++---- admin/tool/mfa/tests/manager_test.php | 21 ++++++++++++ 2 files changed, 64 insertions(+), 6 deletions(-) diff --git a/admin/tool/mfa/classes/manager.php b/admin/tool/mfa/classes/manager.php index 1311b08d890..a63a0c58bf0 100644 --- a/admin/tool/mfa/classes/manager.php +++ b/admin/tool/mfa/classes/manager.php @@ -41,6 +41,18 @@ class manager { /** @var int */ const REDIR_LOOP_THRESHOLD = 5; + /** @var array These components and related fileareas will not redirect. */ + const ALLOWED_COMPONENTS = [ + 'core_admin' => [ + 'logocompact', + 'logo', + 'favicon', + ], + 'tool_mfa' => [ + 'guidance', + ] + ]; + /** * Displays a debug table with current factor information. * @@ -424,6 +436,37 @@ class manager { return self::NO_REDIRECT; } + // Ensure we have a moodle_url object if a string is provided. + if (is_string($url)) { + $url = new \moodle_url($url); + } + + // Check for pluginfile.php urls. + $pluginfileurl = new \moodle_url('/pluginfile.php'); + if ($url->compare($pluginfileurl)) { + // Get the slash arguments. + $args = explode('/', ltrim($url->get_slashargument(), '/')); + + // Remove the contextid because we do not need it for this check. + array_shift($args); + + // Get the component and filearea. + $component = clean_param(array_shift($args), PARAM_COMPONENT); + $filearea = clean_param(array_shift($args), PARAM_AREA); + + // Check allowed components. + if (!array_key_exists($component, static::ALLOWED_COMPONENTS)) { + return self::REDIRECT; + } + + // Check allowed fileareas. + if (!in_array($filearea, static::ALLOWED_COMPONENTS[$component])) { + return self::REDIRECT; + } + + return self::NO_REDIRECT; + } + // Remove all params before comparison. $url->remove_all_params(); @@ -442,12 +485,6 @@ class manager { } } - // Dont redirect logo images from pluginfile.php (for example: logo in header). - $logourl = new \moodle_url('/pluginfile.php/1/core_admin/logocompact/'); - if ($url->compare($logourl)) { - return self::NO_REDIRECT; - } - // Admin not setup. if (!empty($CFG->adminsetuppending)) { return self::NO_REDIRECT; diff --git a/admin/tool/mfa/tests/manager_test.php b/admin/tool/mfa/tests/manager_test.php index 3978ecfa5bb..59396b187f0 100644 --- a/admin/tool/mfa/tests/manager_test.php +++ b/admin/tool/mfa/tests/manager_test.php @@ -261,6 +261,27 @@ final class manager_test extends \advanced_testcase { \core\session\manager::loginas($user2->id, $syscontext, false); $this->assertEquals(\tool_mfa\manager::NO_REDIRECT, \tool_mfa\manager::should_require_mfa($badurl, false)); $this->setUser($user); + + // Access logocompact via pluginfile. + $logourl = new \moodle_url('/pluginfile.php/1/core_admin/logocompact/'); + $this->assertEquals(\tool_mfa\manager::NO_REDIRECT, \tool_mfa\manager::should_require_mfa($logourl, false)); + + // Access logo via pluginfile. + $logourl = new \moodle_url('/pluginfile.php/1/core_admin/logo/'); + $this->assertEquals(\tool_mfa\manager::NO_REDIRECT, \tool_mfa\manager::should_require_mfa($logourl, false)); + + // Access favicon via pluginfile. + $logourl = new \moodle_url('/pluginfile.php/1/core_admin/favicon/'); + $this->assertEquals(\tool_mfa\manager::NO_REDIRECT, \tool_mfa\manager::should_require_mfa($logourl, false)); + + // Access guidance files. + $guideurl = new \moodle_url('/pluginfile.php/1/tool_mfa/guidance/0/capybara.png'); + $this->assertEquals(\tool_mfa\manager::NO_REDIRECT, \tool_mfa\manager::should_require_mfa($guideurl, false)); + + // Access private area. + $user3 = $this->getDataGenerator()->create_user(); + $privateurl = new \moodle_url("/pluginfile.php/{$user3->id}/user/private/privatefile.png"); + $this->assertEquals(\tool_mfa\manager::REDIRECT, \tool_mfa\manager::should_require_mfa($privateurl, false)); } /**