From 56fa860ead7326d186de7779bb6fcb8669295172 Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Fri, 8 Apr 2016 13:01:06 +0800 Subject: [PATCH 1/2] MDL-53772 externallib: Fix busted webservices context handling Fix: $PAGE->context must be reset when calling validate_context Improve: Provide wrapper for calling an external function The wrapper correctly checks the function parameters and return type against the description of the external function, and stores the PAGE and COURSE global state variables, restoring them before the function returns. Fix: buggy unit tests. These tests are expecting debugging from a bug that was fixed, and calling web service functions with no user or session. --- lib/ajax/service.php | 57 +----- lib/externallib.php | 292 ++++++++++++++++++--------- lib/pagelib.php | 23 ++- lib/tests/externallib_test.php | 40 ++++ lib/upgrade.txt | 2 + mod/forum/tests/externallib_test.php | 7 - user/tests/externallib_test.php | 1 + 7 files changed, 258 insertions(+), 164 deletions(-) diff --git a/lib/ajax/service.php b/lib/ajax/service.php index e72f1650ef2..d3d23ab9d77 100644 --- a/lib/ajax/service.php +++ b/lib/ajax/service.php @@ -41,66 +41,15 @@ if ($requests === null) { } $responses = array(); - foreach ($requests as $request) { $response = array(); $methodname = clean_param($request['methodname'], PARAM_ALPHANUMEXT); $index = clean_param($request['index'], PARAM_INT); $args = $request['args']; - try { - $externalfunctioninfo = external_function_info($methodname); - - if (!$externalfunctioninfo->allowed_from_ajax) { - error_log('This external function is not available to ajax. Failed to call "' . $methodname . '"'); - throw new moodle_exception('servicenotavailable', 'webservice'); - } - - // Do not allow access to write or delete webservices as a public user. - if ($externalfunctioninfo->loginrequired) { - if (defined('NO_MOODLE_COOKIES') && NO_MOODLE_COOKIES) { - error_log('Set "loginrequired" to false in db/service.php when calling entry point service-nologin.php. ' . - 'Failed to call "' . $methodname . '"'); - throw new moodle_exception('servicenotavailable', 'webservice'); - } - if (!isloggedin()) { - error_log('This external function is not available to public users. Failed to call "' . $methodname . '"'); - throw new moodle_exception('servicenotavailable', 'webservice'); - } else { - require_sesskey(); - } - } - - // Validate params, this also sorts the params properly, we need the correct order in the next part. - $callable = array($externalfunctioninfo->classname, 'validate_parameters'); - $params = call_user_func($callable, - $externalfunctioninfo->parameters_desc, - $args); - - // Execute - gulp! - $callable = array($externalfunctioninfo->classname, $externalfunctioninfo->methodname); - $result = call_user_func_array($callable, - array_values($params)); - - // Validate the return parameters. - if ($externalfunctioninfo->returns_desc !== null) { - $callable = array($externalfunctioninfo->classname, 'clean_returnvalue'); - $result = call_user_func($callable, $externalfunctioninfo->returns_desc, $result); - } - - $response['error'] = false; - $response['data'] = $result; - $responses[$index] = $response; - } catch (Exception $e) { - $jsonexception = get_exception_info($e); - unset($jsonexception->a); - if (!debugging('', DEBUG_DEVELOPER)) { - unset($jsonexception->debuginfo); - unset($jsonexception->backtrace); - } - $response['error'] = true; - $response['exception'] = $jsonexception; - $responses[$index] = $response; + $response = external_api::call_external_function($methodname, $args, true); + $responses[$index] = $response; + if ($response['error']) { // Do not process the remaining requests. break; } diff --git a/lib/externallib.php b/lib/externallib.php index 002d5727d22..22b07d3df77 100644 --- a/lib/externallib.php +++ b/lib/externallib.php @@ -35,98 +35,7 @@ defined('MOODLE_INTERNAL') || die(); * @since Moodle 2.0 */ function external_function_info($function, $strictness=MUST_EXIST) { - global $DB, $CFG; - - if (!is_object($function)) { - if (!$function = $DB->get_record('external_functions', array('name'=>$function), '*', $strictness)) { - return false; - } - } - - // First try class autoloading. - if (!class_exists($function->classname)) { - // Fallback to explicit include of externallib.php. - $function->classpath = empty($function->classpath) ? core_component::get_component_directory($function->component).'/externallib.php' : $CFG->dirroot.'/'.$function->classpath; - if (!file_exists($function->classpath)) { - throw new coding_exception('Cannot find file with external function implementation: ' . $function->classname); - } - require_once($function->classpath); - if (!class_exists($function->classname)) { - throw new coding_exception('Cannot find external class'); - } - } - - $function->ajax_method = $function->methodname.'_is_allowed_from_ajax'; - $function->parameters_method = $function->methodname.'_parameters'; - $function->returns_method = $function->methodname.'_returns'; - $function->deprecated_method = $function->methodname.'_is_deprecated'; - - // make sure the implementaion class is ok - if (!method_exists($function->classname, $function->methodname)) { - throw new coding_exception('Missing implementation method of '.$function->classname.'::'.$function->methodname); - } - if (!method_exists($function->classname, $function->parameters_method)) { - throw new coding_exception('Missing parameters description'); - } - if (!method_exists($function->classname, $function->returns_method)) { - throw new coding_exception('Missing returned values description'); - } - if (method_exists($function->classname, $function->deprecated_method)) { - if (call_user_func(array($function->classname, $function->deprecated_method)) === true) { - $function->deprecated = true; - } - } - $function->allowed_from_ajax = false; - - // fetch the parameters description - $function->parameters_desc = call_user_func(array($function->classname, $function->parameters_method)); - if (!($function->parameters_desc instanceof external_function_parameters)) { - throw new coding_exception('Invalid parameters description'); - } - - // fetch the return values description - $function->returns_desc = call_user_func(array($function->classname, $function->returns_method)); - // null means void result or result is ignored - if (!is_null($function->returns_desc) and !($function->returns_desc instanceof external_description)) { - throw new coding_exception('Invalid return description'); - } - - //now get the function description - //TODO MDL-31115 use localised lang pack descriptions, it would be nice to have - // easy to understand descriptions in admin UI, - // on the other hand this is still a bit in a flux and we need to find some new naming - // conventions for these descriptions in lang packs - $function->description = null; - $servicesfile = core_component::get_component_directory($function->component).'/db/services.php'; - if (file_exists($servicesfile)) { - $functions = null; - include($servicesfile); - if (isset($functions[$function->name]['description'])) { - $function->description = $functions[$function->name]['description']; - } - if (isset($functions[$function->name]['testclientpath'])) { - $function->testclientpath = $functions[$function->name]['testclientpath']; - } - if (isset($functions[$function->name]['type'])) { - $function->type = $functions[$function->name]['type']; - } - if (isset($functions[$function->name]['ajax'])) { - $function->allowed_from_ajax = $functions[$function->name]['ajax']; - } else if (method_exists($function->classname, $function->ajax_method)) { - if (call_user_func(array($function->classname, $function->ajax_method)) === true) { - debugging('External function ' . $function->ajax_method . '() function is deprecated.' . - 'Set ajax=>true in db/service.php instead.', DEBUG_DEVELOPER); - $function->allowed_from_ajax = true; - } - } - if (isset($functions[$function->name]['loginrequired'])) { - $function->loginrequired = $functions[$function->name]['loginrequired']; - } else { - $function->loginrequired = true; - } - } - - return $function; + return external_api::external_function_info($function, $strictness); } /** @@ -161,6 +70,195 @@ class external_api { /** @var stdClass context where the function calls will be restricted */ private static $contextrestriction; + /** + * Returns detailed function information + * + * @param string|object $function name of external function or record from external_function + * @param int $strictness IGNORE_MISSING means compatible mode, false returned if record not found, debug message if more found; + * MUST_EXIST means throw exception if no record or multiple records found + * @return stdClass description or false if not found or exception thrown + * @since Moodle 2.0 + */ + public static function external_function_info($function, $strictness=MUST_EXIST) { + global $DB, $CFG; + + if (!is_object($function)) { + if (!$function = $DB->get_record('external_functions', array('name' => $function), '*', $strictness)) { + return false; + } + } + + // First try class autoloading. + if (!class_exists($function->classname)) { + // Fallback to explicit include of externallib.php. + if (empty($function->classpath)) { + $function->classpath = core_component::get_component_directory($function->component).'/externallib.php'; + } else { + $function->classpath = $CFG->dirroot.'/'.$function->classpath; + } + if (!file_exists($function->classpath)) { + throw new coding_exception('Cannot find file with external function implementation'); + } + require_once($function->classpath); + if (!class_exists($function->classname)) { + throw new coding_exception('Cannot find external class'); + } + } + + $function->ajax_method = $function->methodname.'_is_allowed_from_ajax'; + $function->parameters_method = $function->methodname.'_parameters'; + $function->returns_method = $function->methodname.'_returns'; + $function->deprecated_method = $function->methodname.'_is_deprecated'; + + // Make sure the implementaion class is ok. + if (!method_exists($function->classname, $function->methodname)) { + throw new coding_exception('Missing implementation method of '.$function->classname.'::'.$function->methodname); + } + if (!method_exists($function->classname, $function->parameters_method)) { + throw new coding_exception('Missing parameters description'); + } + if (!method_exists($function->classname, $function->returns_method)) { + throw new coding_exception('Missing returned values description'); + } + if (method_exists($function->classname, $function->deprecated_method)) { + if (call_user_func(array($function->classname, $function->deprecated_method)) === true) { + $function->deprecated = true; + } + } + $function->allowed_from_ajax = false; + + // Fetch the parameters description. + $function->parameters_desc = call_user_func(array($function->classname, $function->parameters_method)); + if (!($function->parameters_desc instanceof external_function_parameters)) { + throw new coding_exception('Invalid parameters description'); + } + + // Fetch the return values description. + $function->returns_desc = call_user_func(array($function->classname, $function->returns_method)); + // Null means void result or result is ignored. + if (!is_null($function->returns_desc) and !($function->returns_desc instanceof external_description)) { + throw new coding_exception('Invalid return description'); + } + + // Now get the function description. + + // TODO MDL-31115 use localised lang pack descriptions, it would be nice to have + // easy to understand descriptions in admin UI, + // on the other hand this is still a bit in a flux and we need to find some new naming + // conventions for these descriptions in lang packs. + $function->description = null; + $servicesfile = core_component::get_component_directory($function->component).'/db/services.php'; + if (file_exists($servicesfile)) { + $functions = null; + include($servicesfile); + if (isset($functions[$function->name]['description'])) { + $function->description = $functions[$function->name]['description']; + } + if (isset($functions[$function->name]['testclientpath'])) { + $function->testclientpath = $functions[$function->name]['testclientpath']; + } + if (isset($functions[$function->name]['type'])) { + $function->type = $functions[$function->name]['type']; + } + if (isset($functions[$function->name]['ajax'])) { + $function->allowed_from_ajax = $functions[$function->name]['ajax']; + } else if (method_exists($function->classname, $function->ajax_method)) { + if (call_user_func(array($function->classname, $function->ajax_method)) === true) { + debugging('External function ' . $function->ajax_method . '() function is deprecated.' . + 'Set ajax=>true in db/service.php instead.', DEBUG_DEVELOPER); + $function->allowed_from_ajax = true; + } + } + if (isset($functions[$function->name]['loginrequired'])) { + $function->loginrequired = $functions[$function->name]['loginrequired']; + } else { + $function->loginrequired = true; + } + } + + return $function; + } + + /** + * Call an external function validating all params/returns correctly. + * + * Note that an external function may modify the state of the current page, so this wrapper + * saves and restores tha PAGE and COURSE global variables before/after calling the external function. + * + * @param string $function A webservice function name. + * @param array $args Params array (named params) + * @param boolean $ajaxonly If true, an extra check will be peformed to see if ajax is required. + * @return array containing keys for error (bool), exception and data. + */ + public static function call_external_function($function, $args, $ajaxonly=false) { + global $PAGE, $COURSE, $CFG, $SITE; + + require_once($CFG->libdir . "/pagelib.php"); + + $externalfunctioninfo = self::external_function_info($function); + + $currentpage = $PAGE; + $currentcourse = $COURSE; + $response = array(); + + try { + + $PAGE = new moodle_page(); + $COURSE = clone($SITE); + + if ($ajaxonly && !$externalfunctioninfo->allowed_from_ajax) { + throw new moodle_exception('servicenotavailable', 'webservice'); + } + + // Do not allow access to write or delete webservices as a public user. + if ($externalfunctioninfo->loginrequired) { + if (defined('NO_MOODLE_COOKIES') && NO_MOODLE_COOKIES && !PHPUNIT_TEST) { + throw new moodle_exception('servicenotavailable', 'webservice'); + } + if (!isloggedin()) { + throw new moodle_exception('servicenotavailable', 'webservice'); + } else { + require_sesskey(); + } + } + + // Validate params, this also sorts the params properly, we need the correct order in the next part. + $callable = array($externalfunctioninfo->classname, 'validate_parameters'); + $params = call_user_func($callable, + $externalfunctioninfo->parameters_desc, + $args); + + // Execute - gulp! + $callable = array($externalfunctioninfo->classname, $externalfunctioninfo->methodname); + $result = call_user_func_array($callable, + array_values($params)); + + // Validate the return parameters. + if ($externalfunctioninfo->returns_desc !== null) { + $callable = array($externalfunctioninfo->classname, 'clean_returnvalue'); + $result = call_user_func($callable, $externalfunctioninfo->returns_desc, $result); + } + + $response['error'] = false; + $response['data'] = $result; + } catch (Exception $e) { + $exception = get_exception_info($e); + unset($exception->a); + if (!debugging('', DEBUG_DEVELOPER)) { + unset($exception->debuginfo); + unset($exception->backtrace); + } + $response['error'] = true; + $response['exception'] = $exception; + // Do not process the remaining requests. + } + + $PAGE = $currentpage; + $COURSE = $currentcourse; + + return $response; + } + /** * Set context restriction for all following subsequent function calls. * @@ -359,7 +457,7 @@ class external_api { * @since Moodle 2.0 */ public static function validate_context($context) { - global $CFG; + global $CFG, $PAGE; if (empty($context)) { throw new invalid_parameter_exception('Context does not exist'); @@ -382,10 +480,10 @@ class external_api { } } - if ($context->contextlevel >= CONTEXT_COURSE) { - list($context, $course, $cm) = get_context_info_array($context->id); - require_login($course, false, $cm, false, true); - } + $PAGE->reset_theme_and_output(); + list($unused, $course, $cm) = get_context_info_array($context->id); + require_login($course, false, $cm, false, true); + $PAGE->set_context($context); } /** diff --git a/lib/pagelib.php b/lib/pagelib.php index 33466f59f2d..4145f8e7fe7 100644 --- a/lib/pagelib.php +++ b/lib/pagelib.php @@ -981,7 +981,6 @@ class moodle_page { } return; } - // Ideally we should set context only once. if (isset($this->_context) && $context->id !== $this->_context->id) { $current = $this->_context->contextlevel; @@ -993,11 +992,7 @@ class moodle_page { } else { // We do not want devs to do weird switching of context levels on the fly because we might have used // the context already such as in text filter in page title. - // This is explicitly allowed for webservices though which may - // call "external_api::validate_context on many contexts in a single request. - if (!WS_SERVER) { - debugging("Coding problem: unsupported modification of PAGE->context from {$current} to {$context->contextlevel}"); - } + debugging("Coding problem: unsupported modification of PAGE->context from {$current} to {$context->contextlevel}"); } } @@ -1560,6 +1555,22 @@ class moodle_page { $this->_wherethemewasinitialised = debug_backtrace(); } + /** + * Reset the theme and output for a new context. This only makes sense from + * external::validate_context(). Do not cheat. + * + * @return string the name of the theme that should be used on this page. + */ + public function reset_theme_and_output() { + global $COURSE, $SITE; + + $COURSE = clone($SITE); + $this->_theme = null; + $this->_wherethemewasinitialised = null; + $this->_course = null; + $this->_context = null; + } + /** * Work out the theme this page should use. * diff --git a/lib/tests/externallib_test.php b/lib/tests/externallib_test.php index b3b0198046c..e19dd2f6ec4 100644 --- a/lib/tests/externallib_test.php +++ b/lib/tests/externallib_test.php @@ -354,6 +354,46 @@ class core_externallib_testcase extends advanced_testcase { // The extra course passed is not returned. $this->assertArrayNotHasKey($c4->id, $courses); } + + + public function test_call_external_function() { + global $PAGE, $COURSE; + + $this->resetAfterTest(true); + + // Call some webservice functions and verify they are correctly handling $PAGE and $COURSE. + // First test a function that calls validate_context outside a course. + $this->setAdminUser(); + $category = $this->getDataGenerator()->create_category(); + $params = array( + 'contextid' => context_coursecat::instance($category->id)->id, + 'name' => 'aaagrrryyy', + 'idnumber' => '', + 'description' => '' + ); + $cohort1 = $this->getDataGenerator()->create_cohort($params); + $cohort2 = $this->getDataGenerator()->create_cohort(); + + $beforepage = $PAGE; + $beforecourse = $COURSE; + $params = array('cohortids' => array($cohort1->id, $cohort2->id)); + $result = external_api::call_external_function('core_cohort_get_cohorts', $params); + + $this->assertSame($beforepage, $PAGE); + $this->assertSame($beforecourse, $COURSE); + + // Now test a function that calls validate_context inside a course. + $course = $this->getDataGenerator()->create_course(); + + $beforepage = $PAGE; + $beforecourse = $COURSE; + $params = array('courseid' => $course->id, 'options' => array()); + $result = external_api::call_external_function('core_enrol_get_enrolled_users', $params); + + $this->assertSame($beforepage, $PAGE); + $this->assertSame($beforecourse, $COURSE); + } + } /* diff --git a/lib/upgrade.txt b/lib/upgrade.txt index cb4b8aa0f14..43a5ad0e8d5 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -3,6 +3,8 @@ information provided here is intended especially for developers. === 3.1 === +* External functions that are not calling external_api::validate_context are buggy and will now generate + exceptions. Previously they were only generating warnings in the webserver error log. * The moodle/blog:associatecourse and moodle/blog:associatemodule capabilities has been removed. * The following functions has been finally deprecated and can not be used any more: - profile_display_badges() diff --git a/mod/forum/tests/externallib_test.php b/mod/forum/tests/externallib_test.php index a7ce2b040fa..371b664962d 100644 --- a/mod/forum/tests/externallib_test.php +++ b/mod/forum/tests/externallib_test.php @@ -346,10 +346,6 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { $discussions = mod_forum_external::get_forum_discussions(array($forum1->id, $forum2->id)); $discussions = external_api::clean_returnvalue(mod_forum_external::get_forum_discussions_returns(), $discussions); $this->assertEquals($expecteddiscussions, $discussions); - // Some debugging is going to be produced, this is because we switch PAGE contexts in the get_forum_discussions function, - // the switch happens when the validate_context function is called inside a foreach loop. - // See MDL-41746 for more information. - $this->assertDebuggingCalled(); // Remove the users post from the qanda forum and ensure they can still see the discussion. $DB->delete_records('forum_posts', array('id' => $discussion2reply1->id)); @@ -365,7 +361,6 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { } catch (moodle_exception $e) { $this->assertEquals('nopermissions', $e->errorcode); } - $this->assertDebuggingCalled(); // Unenrol user from second course. $enrol->unenrol_user($instance2, $user1->id); @@ -857,8 +852,6 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { mod_forum_external::add_discussion_post($discussion->firstpost, 'some subject', 'some text here...'); $this->fail('Exception expected due to invalid permissions for posting.'); } catch (moodle_exception $e) { - // Expect debugging since we are switching context, and this is something WS_SERVER mode don't like. - $this->assertDebuggingCalled(); $this->assertEquals('nopostforum', $e->errorcode); } diff --git a/user/tests/externallib_test.php b/user/tests/externallib_test.php index ebbb290089a..6172453b15d 100644 --- a/user/tests/externallib_test.php +++ b/user/tests/externallib_test.php @@ -818,6 +818,7 @@ class core_user_externallib_testcase extends externallib_advanced_testcase { $device2['pushid'] = "0987654321"; $device2['appid'] = "other.app.com"; + $this->setAdminUser(); // Create a user device using the external API function. core_user_external::add_user_device($device['appid'], $device['name'], $device['model'], $device['platform'], $device['version'], $device['pushid'], $device['uuid']); From 787ec240c110d4929e29aeb56fc6d7e8bafc24b0 Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Tue, 12 Apr 2016 12:39:23 +0800 Subject: [PATCH 2/2] MDL-53772 inplace_editable: set context in callbacks --- cohort/classes/output/cohortidnumber.php | 1 + cohort/classes/output/cohortname.php | 1 + course/classes/output/course_module_name.php | 10 +++++----- course/format/lib.php | 2 +- lib/external/externallib.php | 1 - tag/classes/external.php | 1 - tag/lib.php | 1 + tag/tests/external_test.php | 1 + 8 files changed, 10 insertions(+), 8 deletions(-) diff --git a/cohort/classes/output/cohortidnumber.php b/cohort/classes/output/cohortidnumber.php index 56289174e41..454fdde638b 100644 --- a/cohort/classes/output/cohortidnumber.php +++ b/cohort/classes/output/cohortidnumber.php @@ -61,6 +61,7 @@ class cohortidnumber extends \core\output\inplace_editable { global $DB; $cohort = $DB->get_record('cohort', array('id' => $cohortid), '*', MUST_EXIST); $cohortcontext = \context::instance_by_id($cohort->contextid); + \external_api::validate_context($cohortcontext); require_capability('moodle/cohort:manage', $cohortcontext); $record = (object)array('id' => $cohort->id, 'idnumber' => $newvalue, 'contextid' => $cohort->contextid); cohort_update_cohort($record); diff --git a/cohort/classes/output/cohortname.php b/cohort/classes/output/cohortname.php index 7c748094c16..7ac5950ebf9 100644 --- a/cohort/classes/output/cohortname.php +++ b/cohort/classes/output/cohortname.php @@ -61,6 +61,7 @@ class cohortname extends \core\output\inplace_editable { global $DB; $cohort = $DB->get_record('cohort', array('id' => $cohortid), '*', MUST_EXIST); $cohortcontext = \context::instance_by_id($cohort->contextid); + \external_api::validate_context($cohortcontext); require_capability('moodle/cohort:manage', $cohortcontext); $newvalue = clean_param($newvalue, PARAM_TEXT); if (strval($newvalue) !== '') { diff --git a/course/classes/output/course_module_name.php b/course/classes/output/course_module_name.php index e286e555b2d..160369bf2d2 100644 --- a/course/classes/output/course_module_name.php +++ b/course/classes/output/course_module_name.php @@ -86,15 +86,15 @@ class course_module_name extends \core\output\inplace_editable { * @return static */ public static function update($itemid, $newvalue) { - list($course, $cm) = get_course_and_cm_from_cmid($itemid); - $context = context_module::instance($cm->id); + global $PAGE; + $context = context_module::instance($itemid); // Check access. - require_login($course, false, $cm, true, true); + \external_api::validate_context($context); require_capability('moodle/course:manageactivities', $context); // Update value. - set_coursemodule_name($cm->id, $newvalue); + set_coursemodule_name($PAGE->cm->id, $newvalue); // Return instance. - $cm = get_fast_modinfo($course)->get_cm($cm->id); + $cm = get_fast_modinfo($PAGE->course)->get_cm($PAGE->cm->id); return new static($cm, true); } } diff --git a/course/format/lib.php b/course/format/lib.php index 8a4078c0d32..208c1881455 100644 --- a/course/format/lib.php +++ b/course/format/lib.php @@ -1089,8 +1089,8 @@ abstract class format_base { */ public function inplace_editable_update_section_name($section, $itemtype, $newvalue) { if ($itemtype === 'sectionname' || $itemtype === 'sectionnamenl') { - require_login($section->course, false, null, true, true); $context = context_course::instance($section->course); + external_api::validate_context($context); require_capability('moodle/course:update', $context); $newtitle = clean_param($newvalue, PARAM_TEXT); diff --git a/lib/external/externallib.php b/lib/external/externallib.php index 6def1cf5f18..179448293cc 100644 --- a/lib/external/externallib.php +++ b/lib/external/externallib.php @@ -386,7 +386,6 @@ class core_external extends external_api { if (!$tmpl || !($tmpl instanceof \core\output\inplace_editable)) { throw new \moodle_exception('inplaceeditableerror'); } - $PAGE->set_context(null); // To prevent warning if context was not set in the callback. return $tmpl->export_for_template($PAGE->get_renderer('core')); } diff --git a/tag/classes/external.php b/tag/classes/external.php index c8fa037f0a0..7b38b6f12fa 100644 --- a/tag/classes/external.php +++ b/tag/classes/external.php @@ -302,7 +302,6 @@ class core_tag_external extends external_api { $context = $params['ctx'] ? context::instance_by_id($params['ctx']) : context_system::instance(); require_login(null, false, null, false, true); self::validate_context($context); - $PAGE->set_context(null); $tag = core_tag_tag::get_by_name($params['tc'], $params['tag'], '*', MUST_EXIST); $tagareas = core_tag_collection::get_areas($params['tc']); diff --git a/tag/lib.php b/tag/lib.php index 77f1a79f0a3..2d5e2dddbf9 100644 --- a/tag/lib.php +++ b/tag/lib.php @@ -50,6 +50,7 @@ function tag_page_type_list($pagetype, $parentcontext, $currentcontext) { * @return \core\output\inplace_editable */ function core_tag_inplace_editable($itemtype, $itemid, $newvalue) { + \external_api::validate_context(context_system::instance()); if ($itemtype === 'tagname') { return \core_tag\output\tagname::update($itemid, $newvalue); } else if ($itemtype === 'tagareaenable') { diff --git a/tag/tests/external_test.php b/tag/tests/external_test.php index bf418e97440..b4fa7cdd7e7 100644 --- a/tag/tests/external_test.php +++ b/tag/tests/external_test.php @@ -157,6 +157,7 @@ class core_tag_external_testcase extends externallib_advanced_testcase { $this->resetAfterTest(true); $tag = $this->getDataGenerator()->create_tag(); + $this->setUser($this->getDataGenerator()->create_user()); // Call service for core_tag component without necessary permissions. try {