From 55ab5d67588ea8a419e98274f034b3e208cbe176 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 13 Jan 2026 21:19:11 +0800 Subject: [PATCH] MDL-87648 router: Add check report data for router configuration --- .github/workflows/config-template.php | 2 + admin/cli/checks.php | 4 +- public/admin/environment.xml | 1 + public/lang/en/admin.php | 16 ++ .../lib/classes/check/environment/router.php | 158 ++++++++++++++++++ public/lib/classes/check/manager.php | 1 + public/lib/classes/environment.php | 19 +++ .../route/controller/test_controller.php | 45 +++++ .../classes/route/shim/test_controller.php | 75 +++++++++ public/lib/exampleshimroute.php | 30 ++++ 10 files changed, 349 insertions(+), 2 deletions(-) create mode 100644 public/lib/classes/check/environment/router.php create mode 100644 public/lib/classes/route/controller/test_controller.php create mode 100644 public/lib/classes/route/shim/test_controller.php create mode 100644 public/lib/exampleshimroute.php diff --git a/.github/workflows/config-template.php b/.github/workflows/config-template.php index 2227832ee3b..ae49d583954 100644 --- a/.github/workflows/config-template.php +++ b/.github/workflows/config-template.php @@ -58,6 +58,8 @@ $CFG->pathtophp = getenv('pathtophp'); $CFG->phpunit_dataroot = realpath(dirname(__DIR__)) . '/phpunitdata'; $CFG->phpunit_prefix = 't_'; +$CFG->routerconfigured = true; + define('TEST_EXTERNAL_FILES_HTTP_URL', 'http://localhost:8080'); define('TEST_EXTERNAL_FILES_HTTPS_URL', 'http://localhost:8080'); diff --git a/admin/cli/checks.php b/admin/cli/checks.php index 40deb4d5535..89a51a9a7ba 100644 --- a/admin/cli/checks.php +++ b/admin/cli/checks.php @@ -142,7 +142,8 @@ foreach ($checks as $check) { ); $summary = str_replace("\n", "\n" . $prefix . ' ', $summary); - $output .= sprintf( $format, '', ' ' . $summary); + $output .= sprintf($format, '', ' ' . $summary); + $output .= sprintf($format, '', ' ' . html_to_text($result->get_details(), width: 0, dolinks: false)); if ($options['verbose']) { $actionlink = $check->get_action_link(); @@ -168,4 +169,3 @@ if ($output) { // NRPE shell exit code. exit($exitcode); - diff --git a/public/admin/environment.xml b/public/admin/environment.xml index 30a56c7abe5..4593b6e3a20 100644 --- a/public/admin/environment.xml +++ b/public/admin/environment.xml @@ -5233,6 +5233,7 @@ + diff --git a/public/lang/en/admin.php b/public/lang/en/admin.php index 88b7969d33e..c3f4aab139d 100644 --- a/public/lang/en/admin.php +++ b/public/lang/en/admin.php @@ -126,6 +126,7 @@ $string['cannotuninstall'] = '{$a} can not be uninstalled.'; $string['categoryemail'] = 'Email'; $string['cfgwwwrootslashwarning'] = '$CFG->wwwroot is defined incorrectly in the config.php file. It includes a \'/\' character at the end which must be removed.'; $string['cfgwwwrootwarning'] = '$CFG->wwwroot is defined incorrectly in the config.php file. It should match the URL you are using to access this page.'; +$string['check_router'] = 'Router configuration'; $string['checkupgradepending'] = 'Upgrade'; $string['cleanup'] = 'Cleanup'; $string['clianswerno'] = 'n'; @@ -1171,6 +1172,21 @@ $string['requiremodintro'] = 'Require activity description'; $string['requiremodintro_desc'] = 'If enabled, users will be forced to enter a description for each activity.'; $string['required'] = 'Required'; $string['requires'] = 'Requires'; +$string['routerconfigurationset'] = 'Moodle has been informed that the web server is correctly configured.'; +$string['routerconfigureddetails'] = 'The router correctly serves {$a->url} with a {$a->expectedstatuscode} ("{$a->expectedstatuscodetitle}") response.'; +$string['routerconfiguredok'] = 'The router appears to be configured correctly.'; +$string['routerconfiguredwithissues'] = 'The router is not correctly configured.'; +$string['routerconfiguredwithissuesdetail'] = 'The router is not correctly configured - Issues were detected in {$a->count} checks.'; +$string['routerdocs'] = 'Router documentation'; +$string['routerexpectedgot'] = 'Expected {$a->expectedstatuscode} ("{$a->expectedstatuscodetitle}") and received {$a->statuscode} ("{$a->statuscodetitle}").'; +$string['routerfailapipage'] = 'An API route did not respond with the expected response.'; +$string['routerfailmissingpage'] = 'The missing page handler did not respond to the test URL correctly.'; +$string['routerfailpage'] = 'A standard page controller could not be loaded correctly. The router may not be configured properly.'; +$string['routerfailroutershim'] = 'A routed page whose path ends in ".php" did not response correctly.'; +$string['routerfailshim'] = 'A shimmed page did not response correctly.'; +$string['routernotconfigured'] = 'The router is not configured.'; +$string['routernotconfigureddetail'] = 'The router has not been configured correctly. Please see the documentation for more information on how to configure it correctly.'; +$string['routernotconfigureddetailwithurl'] = 'The following URL did not return the expected status code: {$a->url}.'; $string['purgecaches'] = 'Purge all caches'; $string['purgecachesconfirm'] = 'Moodle can cache themes, JavaScript, language strings, filtered text, RSS feeds and other pieces of calculated data. Purging caches will delete data from the server and force browsers to re-fetch data, so you can be sure you are seeing the most up-to-date values produced by the current code. There is no danger in purging caches, but your site may appear slower for a while until the server and clients calculate new information and cache it.'; $string['purgecachesfinished'] = 'All caches were purged.'; diff --git a/public/lib/classes/check/environment/router.php b/public/lib/classes/check/environment/router.php new file mode 100644 index 00000000000..2b5562f0523 --- /dev/null +++ b/public/lib/classes/check/environment/router.php @@ -0,0 +1,158 @@ +. + +namespace core\check\environment; + +use core\check\check; +use core\check\result; +use core\output\action_link; +use GuzzleHttp\HandlerStack; + +/** + * Checks status of router by making test requests to shimmed and API pages. + * + * @package core + * @category check + * @copyright Andrew Lyons + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class router extends check { + #[\Override] + public function get_name(): string { + return get_string('check_router', 'admin'); + } + + /** + * Get the list of tests + * + * @return \Generator + */ + protected function get_tests(): \Generator { + yield [ + 'url' => '/core/check/controller/test', + 'expectedcode' => 200, + 'expectedcodetitle' => "OK", + 'failfeedbackstr' => 'routerfailpage', + 'bookmarkanchor' => 'routerfailpage', + ]; + yield [ + 'url' => '/api/rest/v2/openapi.json', + 'expectedcode' => 200, + 'expectedcodetitle' => "OK", + 'failfeedbackstr' => 'routerfailapipage', + 'bookmarkanchor' => 'routerfailapipage', + ]; + yield [ + 'url' => '/not/a/valid/request', + 'expectedcode' => 404, + 'expectedcodetitle' => "Not Found", + 'failfeedbackstr' => 'routerfailmissingpage', + 'bookmarkanchor' => 'routerfailmissingpage', + ]; + yield [ + 'url' => \core\router\util::get_path_for_callable([\core\route\shim\test_controller::class, 'real_file_shim']), + 'expectedcode' => 302, + 'expectedcodetitle' => "Found", + 'failfeedbackstr' => 'routerfailshim', + 'bookmarkanchor' => 'routerfailshim', + ]; + yield [ + 'url' => \core\router\util::get_path_for_callable([\core\route\shim\test_controller::class, 'nofile_shim']), + 'expectedcode' => 302, + 'expectedcodetitle' => "Found", + 'failfeedbackstr' => 'routerfailroutershim', + 'bookmarkanchor' => 'routerfailroutershim', + ]; + } + + #[\Override] + public function get_result(): result { + global $CFG; + + if (empty($CFG->routerconfigured)) { + $result = new result( + result::ERROR, + get_string('routernotconfigured', 'admin'), + get_string('routernotconfigureddetail', 'admin', [ + 'docs' => get_docs_url("Configuring_the_Router#routernotconfigured"), + ]), + ); + + return $result; + } + + // The router is marked as configured. Check if it actually works though. + $client = \core\di::get(\core\http_client::class); + + $clientoptions = [ + 'http_errors' => false, + 'allow_redirects' => false, + // Override the Handler Stack to ensure that no caching is used, an that the security helper is not applied. + 'handler' => HandlerStack::create(), + ]; + + foreach ($this->get_tests() as $test) { + if ($test['url'] instanceof \core\url) { + $fullurl = $test['url']->out(false); + } else { + $fullurl = $CFG->wwwroot . $test['url']; + } + + try { + $response = $client->get($fullurl, $clientoptions); + $code = $response->getStatusCode(); + $codetitle = $response->getReasonPhrase(); + } catch (\GuzzleHttp\Exception\GuzzleException $e) { + $code = $e->getCode(); + $codetitle = $e->getMessage(); + } + + $resultprops = [ + 'url' => $fullurl, + 'docs' => get_docs_url("Configuring_the_Router#{$test['bookmarkanchor']}"), + 'expectedstatuscode' => $test['expectedcode'], + 'expectedstatuscodetitle' => $test['expectedcodetitle'], + 'statuscode' => $code, + 'statuscodetitle' => $codetitle, + ]; + if ($code !== $test['expectedcode']) { + $expectedgot = get_string('routerexpectedgot', 'admin', $resultprops); + return new result( + result::ERROR, + get_string($test['failfeedbackstr'], 'admin', $resultprops), + get_string('routernotconfigureddetailwithurl', 'admin', $resultprops) . " {$expectedgot}", + new action_link( + new \core\url(get_docs_url("Configuring_the_Router#{$test['bookmarkanchor']}")), + get_string('routerdocs', 'admin'), + ), + ); + } + } + + return new result( + result::OK, + get_string('routerconfiguredok', 'admin'), + ); + } + + #[\Override] + public function get_action_link(): ?action_link { + return new action_link( + new \core\url(get_docs_url('Configuring_the_Router')), + get_string('routerdocs', 'admin'), + ); + } +} diff --git a/public/lib/classes/check/manager.php b/public/lib/classes/check/manager.php index fceea2ff6f9..4b04fc0e03b 100644 --- a/public/lib/classes/check/manager.php +++ b/public/lib/classes/check/manager.php @@ -97,6 +97,7 @@ class manager { new environment\environment(), new environment\upgradecheck(), new environment\antivirus(), + new environment\router(), ]; // Any plugin can add status checks to this report by implementing a callback diff --git a/public/lib/classes/environment.php b/public/lib/classes/environment.php index c05e756ef4c..403dc7357f4 100644 --- a/public/lib/classes/environment.php +++ b/public/lib/classes/environment.php @@ -160,4 +160,23 @@ class environment { return !empty($CFG->debugdeveloper); } + + /** + * Ensure that the Router is correctly configured. + * + * @param \environment_results $result + * @return \environment_results|null + */ + public static function check_router_configuration(\environment_results $result): ?\environment_results { + global $CFG; + + if (empty($CFG->routerconfigured)) { + // The router has not been marked as configured. + $result->setInfo('Router not configured'); + $result->setFeedbackStr('routernotconfigured'); + return $result; + } + + return null; + } } diff --git a/public/lib/classes/route/controller/test_controller.php b/public/lib/classes/route/controller/test_controller.php new file mode 100644 index 00000000000..f0b972c7d1b --- /dev/null +++ b/public/lib/classes/route/controller/test_controller.php @@ -0,0 +1,45 @@ +. + +namespace core\route\controller; + +use Psr\Http\Message\ResponseInterface; + +/** + * Test controller to provide a route for testing purposes. + * + * This set of routes is primarily intended for use with the Environment Checks + * to help administrators ensure that the Routing system is correctly configured. + * + * @package core + * @copyright Andrew Lyons + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class test_controller { + /** + * Test controller route. + * + * @param ResponseInterface $response + * @return ResponseInterface + */ + #[\core\router\route( + path: '/check/controller/test', + )] + public function test_action(ResponseInterface $response): ResponseInterface { + $response->getBody()->write('Test controller response'); + return $response; + } +} diff --git a/public/lib/classes/route/shim/test_controller.php b/public/lib/classes/route/shim/test_controller.php new file mode 100644 index 00000000000..c820ae60e89 --- /dev/null +++ b/public/lib/classes/route/shim/test_controller.php @@ -0,0 +1,75 @@ +. + +namespace core\route\shim; + +use core\router\route_controller; +use Psr\Http\Message\RequestInterface; +use Psr\Http\Message\ResponseInterface; + +/** + * An example shim route to use for testing. + * + * This set of routes is primarily intended for use with the Environment Checks + * to help administrators ensure that the Routing system is correctly configured. + * + * @package core + * @copyright Andrew Lyons + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class test_controller { + use route_controller; + + /** + * An example shim route action for a file which manually shims the request. + * + * @param ResponseInterface $response + * @return ResponseInterface + */ + #[\core\router\route( + path: '/lib/exampleshimroute.php', + )] + public function real_file_shim( + RequestInterface $request, + ResponseInterface $response, + ): ResponseInterface { + return self::redirect_to_callable( + $request, + $response, + [\core\route\controller\test_controller::class, 'test_action'], + ); + } + + /** + * An example shim route action for a file which no longer exists. + * + * @param ResponseInterface $response + * @return ResponseInterface + */ + #[\core\router\route( + path: '/lib/exampleshimroute2.php', + )] + public function nofile_shim( + RequestInterface $request, + ResponseInterface $response, + ): ResponseInterface { + return self::redirect_to_callable( + $request, + $response, + [\core\route\controller\test_controller::class, 'test_action'], + ); + } +} diff --git a/public/lib/exampleshimroute.php b/public/lib/exampleshimroute.php new file mode 100644 index 00000000000..bc48c9353ba --- /dev/null +++ b/public/lib/exampleshimroute.php @@ -0,0 +1,30 @@ +. + +/** + * An example shim route to use for testing. + * + * This page is primarily intended for use with the Environment Checks + * to help administrators ensure that the Routing system is correctly configured. + * + * This file simulates a legacy file which has not yet been removed, but which exists in the Routing configuration. + * + * @package core + * @copyright Andrew Lyons + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +require_once("../r.php");