From 2479a7c4464ee248455b94f32f17dbade078ca11 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Tue, 24 Oct 2017 13:24:50 +0200 Subject: [PATCH 1/2] MDL-60572 admin: Fix forgottenpasswordurl for WS We should expect URLs in that field. The tool_mobile change is to not break the WS response if forgottenpasswordurl does not contain a URL. --- admin/tool/mobile/classes/api.php | 2 +- admin/tool/mobile/tests/externallib_test.php | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/admin/tool/mobile/classes/api.php b/admin/tool/mobile/classes/api.php index bfc42ee0226..9b3e3b3a0a5 100644 --- a/admin/tool/mobile/classes/api.php +++ b/admin/tool/mobile/classes/api.php @@ -140,7 +140,7 @@ class api { 'rememberusername' => $CFG->rememberusername, 'authloginviaemail' => $CFG->authloginviaemail, 'registerauth' => $CFG->registerauth, - 'forgottenpasswordurl' => $CFG->forgottenpasswordurl, + 'forgottenpasswordurl' => clean_param($CFG->forgottenpasswordurl, PARAM_URL), // We may expect a mailto: here. 'authinstructions' => $authinstructions, 'authnoneenabled' => (int) is_enabled_auth('none'), 'enablewebservices' => $CFG->enablewebservices, diff --git a/admin/tool/mobile/tests/externallib_test.php b/admin/tool/mobile/tests/externallib_test.php index b5986aa9d9a..ff1f3c1a829 100644 --- a/admin/tool/mobile/tests/externallib_test.php +++ b/admin/tool/mobile/tests/externallib_test.php @@ -97,11 +97,13 @@ class tool_mobile_external_testcase extends externallib_advanced_testcase { set_config('typeoflogin', api::LOGIN_VIA_BROWSER, 'tool_mobile'); set_config('logo', 'mock.png', 'core_admin'); set_config('logocompact', 'mock.png', 'core_admin'); + set_config('forgottenpasswordurl', 'mailto:fake@email.zy'); // Test old hack. list($authinstructions, $notusedformat) = external_format_text($authinstructions, FORMAT_MOODLE, $context->id); $expected['registerauth'] = 'email'; $expected['authinstructions'] = $authinstructions; $expected['typeoflogin'] = api::LOGIN_VIA_BROWSER; + $expected['forgottenpasswordurl'] = ''; // Expect empty when it's not an URL. if ($logourl = $OUTPUT->get_logo_url()) { $expected['logourl'] = $logourl->out(false); From 6078d420bf1749f16dac0b4a7c07d78ae62a6fd5 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Fri, 27 Oct 2017 10:34:49 +0200 Subject: [PATCH 2/2] MDL-60572 admin: Enforce URLs in forgottenpasswordurl setting Also display warnings for admins. --- admin/index.php | 3 ++- admin/renderer.php | 22 +++++++++++++++++++++- admin/settings/plugins.php | 2 +- lang/en/admin.php | 1 + 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/admin/index.php b/admin/index.php index 8d0199ee1dd..e04c3197efb 100644 --- a/admin/index.php +++ b/admin/index.php @@ -868,6 +868,7 @@ $cachewarnings = cache_helper::warnings(); $eventshandlers = $DB->get_records_sql('SELECT DISTINCT component FROM {events_handlers}'); $themedesignermode = !empty($CFG->themedesignermode); $mobileconfigured = !empty($CFG->enablemobilewebservice); +$invalidforgottenpasswordurl = !empty($CFG->forgottenpasswordurl) && empty(clean_param($CFG->forgottenpasswordurl, PARAM_URL)); // Check if a directory with development libraries exists. if (empty($CFG->disabledevlibdirscheck) && (is_dir($CFG->dirroot.'/vendor') || is_dir($CFG->dirroot.'/node_modules'))) { @@ -885,4 +886,4 @@ $output = $PAGE->get_renderer('core', 'admin'); echo $output->admin_notifications_page($maturity, $insecuredataroot, $errorsdisplayed, $cronoverdue, $dbproblems, $maintenancemode, $availableupdates, $availableupdatesfetch, $buggyiconvnomb, $registered, $cachewarnings, $eventshandlers, $themedesignermode, $devlibdir, - $mobileconfigured, $overridetossl); + $mobileconfigured, $overridetossl, $invalidforgottenpasswordurl); diff --git a/admin/renderer.php b/admin/renderer.php index 9ea4ea1d280..dbd77f6122e 100644 --- a/admin/renderer.php +++ b/admin/renderer.php @@ -280,6 +280,7 @@ class core_admin_renderer extends plugin_renderer_base { * @param bool $devlibdir Warn about development libs directory presence. * @param bool $mobileconfigured Whether the mobile web services have been enabled * @param bool $overridetossl Whether or not ssl is being forced. + * @param bool $invalidforgottenpasswordurl Whether the forgotten password URL does not link to a valid URL. * * @return string HTML to output. */ @@ -287,7 +288,7 @@ class core_admin_renderer extends plugin_renderer_base { $cronoverdue, $dbproblems, $maintenancemode, $availableupdates, $availableupdatesfetch, $buggyiconvnomb, $registered, array $cachewarnings = array(), $eventshandlers = 0, $themedesignermode = false, $devlibdir = false, $mobileconfigured = false, - $overridetossl = false) { + $overridetossl = false, $invalidforgottenpasswordurl = false) { global $CFG; $output = ''; @@ -308,6 +309,7 @@ class core_admin_renderer extends plugin_renderer_base { $output .= $this->events_handlers($eventshandlers); $output .= $this->registration_warning($registered); $output .= $this->mobile_configuration_warning($mobileconfigured); + $output .= $this->forgotten_password_url_warning($invalidforgottenpasswordurl); ////////////////////////////////////////////////////////////////////////////////////////////////// //// IT IS ILLEGAL AND A VIOLATION OF THE GPL TO HIDE, REMOVE OR MODIFY THIS COPYRIGHT NOTICE /// @@ -866,6 +868,24 @@ class core_admin_renderer extends plugin_renderer_base { return $output; } + /** + * Display a warning about the forgotten password URL not linking to a valid URL. + * + * @param boolean $invalidforgottenpasswordurl true if the forgotten password URL is not valid + * @return string HTML to output. + */ + protected function forgotten_password_url_warning($invalidforgottenpasswordurl) { + $output = ''; + if ($invalidforgottenpasswordurl) { + $settingslink = new moodle_url('/admin/settings.php', ['section' => 'manageauths']); + $configurebutton = $this->single_button($settingslink, get_string('check', 'moodle')); + $output .= $this->warning(get_string('invalidforgottenpasswordurl', 'admin') . ' ' . $configurebutton, + 'error alert alert-danger'); + } + + return $output; + } + /** * Helper method to render the information about the available Moodle update * diff --git a/admin/settings/plugins.php b/admin/settings/plugins.php index 4444ff06444..c55b1f12dc3 100644 --- a/admin/settings/plugins.php +++ b/admin/settings/plugins.php @@ -102,7 +102,7 @@ if ($hassiteconfig) { $temp->add(new admin_setting_configtext('alternateloginurl', new lang_string('alternateloginurl', 'auth'), new lang_string('alternatelogin', 'auth', htmlspecialchars(get_login_url())), '')); $temp->add(new admin_setting_configtext('forgottenpasswordurl', new lang_string('forgottenpasswordurl', 'auth'), - new lang_string('forgottenpassword', 'auth'), '')); + new lang_string('forgottenpassword', 'auth'), '', PARAM_URL)); $temp->add(new admin_setting_confightmleditor('auth_instructions', new lang_string('instructions', 'auth'), new lang_string('authinstructions', 'auth'), '')); $setting = new admin_setting_configtext('allowemailaddresses', new lang_string('allowemailaddresses', 'admin'), diff --git a/lang/en/admin.php b/lang/en/admin.php index 89832944e8d..68262c80488 100644 --- a/lang/en/admin.php +++ b/lang/en/admin.php @@ -616,6 +616,7 @@ $string['installhijacked'] = 'Installation must be finished from the original IP $string['installsessionerror'] = 'Can not initialise PHP session, please verify that your browser accepts cookies.'; $string['intlrecommended'] = 'Intl extension is used to improve internationalization support, such as locale aware sorting.'; $string['intlrequired'] = 'Intl extension is required to improve internationalization support, such as locale aware sorting and international domain names.'; +$string['invalidforgottenpasswordurl'] = 'The forgotten password URL is not a valid URL.'; $string['invalidsection'] = 'Invalid section.'; $string['invaliduserchangeme'] = 'Username "changeme" is reserved -- you cannot create an account with it.'; $string['ipblocked'] = 'This site is not available currently.';