From 2bf6db2d5a0e3ad46dbf036960fb2fcef0cd7962 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Tue, 21 Jan 2020 10:38:23 +0100 Subject: [PATCH 01/15] MDL-67748 admin: Move the web services under the server administration I suppose the original reasoning was that the web services protocols are pluggable and that plugins can provide external function. But those are not good reasons to put the whole Web services section under plugins. Web services are not a plugin type and they always felt very counter-intuitive there. From the server administrtaion perspective, web services are similar to scheduled tasks and therefore, they should live in the same section. --- admin/settings/plugins.php | 49 ----------------- admin/settings/server.php | 72 +++++++++++++++++++++++++ admin/tests/behat/manage_tokens.feature | 2 +- admin/upgrade.txt | 3 ++ webservice/lib.php | 2 +- 5 files changed, 77 insertions(+), 51 deletions(-) diff --git a/admin/settings/plugins.php b/admin/settings/plugins.php index 8def3d77223..883539b957c 100644 --- a/admin/settings/plugins.php +++ b/admin/settings/plugins.php @@ -401,55 +401,6 @@ if ($hassiteconfig) { /** @var \core\plugininfo\repository $plugin */ $plugin->load_settings($ADMIN, 'repositorysettings', $hassiteconfig); } - -/// Web services - $ADMIN->add('modules', new admin_category('webservicesettings', new lang_string('webservices', 'webservice'))); - - /// overview page - $temp = new admin_settingpage('webservicesoverview', new lang_string('webservicesoverview', 'webservice')); - $temp->add(new admin_setting_webservicesoverview()); - $ADMIN->add('webservicesettings', $temp); - //API documentation - $ADMIN->add('webservicesettings', new admin_externalpage('webservicedocumentation', new lang_string('wsdocapi', 'webservice'), "$CFG->wwwroot/$CFG->admin/webservice/documentation.php", 'moodle/site:config', false)); - /// manage service - $temp = new admin_settingpage('externalservices', new lang_string('externalservices', 'webservice')); - $temp->add(new admin_setting_heading('manageserviceshelpexplaination', new lang_string('information', 'webservice'), new lang_string('servicehelpexplanation', 'webservice'))); - $temp->add(new admin_setting_manageexternalservices()); - $ADMIN->add('webservicesettings', $temp); - $ADMIN->add('webservicesettings', new admin_externalpage('externalservice', new lang_string('editaservice', 'webservice'), "$CFG->wwwroot/$CFG->admin/webservice/service.php", 'moodle/site:config', true)); - $ADMIN->add('webservicesettings', new admin_externalpage('externalservicefunctions', new lang_string('externalservicefunctions', 'webservice'), "$CFG->wwwroot/$CFG->admin/webservice/service_functions.php", 'moodle/site:config', true)); - $ADMIN->add('webservicesettings', new admin_externalpage('externalserviceusers', new lang_string('externalserviceusers', 'webservice'), "$CFG->wwwroot/$CFG->admin/webservice/service_users.php", 'moodle/site:config', true)); - $ADMIN->add('webservicesettings', new admin_externalpage('externalserviceusersettings', new lang_string('serviceusersettings', 'webservice'), "$CFG->wwwroot/$CFG->admin/webservice/service_user_settings.php", 'moodle/site:config', true)); - /// manage protocol page link - $temp = new admin_settingpage('webserviceprotocols', new lang_string('manageprotocols', 'webservice')); - $temp->add(new admin_setting_managewebserviceprotocols()); - if (empty($CFG->enablewebservices)) { - $temp->add(new admin_setting_heading('webservicesaredisabled', '', new lang_string('disabledwarning', 'webservice'))); - } - - // We cannot use $OUTPUT this early, doing so means that we lose the ability - // to set the page layout on all admin pages. - // $wsdoclink = $OUTPUT->doc_link('How_to_get_a_security_key'); - $url = new moodle_url(get_docs_url('How_to_get_a_security_key')); - $wsdoclink = html_writer::tag('a', new lang_string('supplyinfo', 'webservice'), array('href'=>$url)); - $temp->add(new admin_setting_configcheckbox('enablewsdocumentation', new lang_string('enablewsdocumentation', - 'admin'), new lang_string('configenablewsdocumentation', 'admin', $wsdoclink), false)); - $ADMIN->add('webservicesettings', $temp); - /// links to protocol pages - $plugins = core_plugin_manager::instance()->get_plugins_of_type('webservice'); - core_collator::asort_objects_by_property($plugins, 'displayname'); - foreach ($plugins as $plugin) { - /** @var \core\plugininfo\webservice $plugin */ - $plugin->load_settings($ADMIN, 'webservicesettings', $hassiteconfig); - } - /// manage token page link - $ADMIN->add('webservicesettings', new admin_externalpage('addwebservicetoken', new lang_string('managetokens', 'webservice'), "$CFG->wwwroot/$CFG->admin/webservice/tokens.php", 'moodle/site:config', true)); - $temp = new admin_settingpage('webservicetokens', new lang_string('managetokens', 'webservice')); - $temp->add(new admin_setting_managewebservicetokens()); - if (empty($CFG->enablewebservices)) { - $temp->add(new admin_setting_heading('webservicesaredisabled', '', new lang_string('disabledwarning', 'webservice'))); - } - $ADMIN->add('webservicesettings', $temp); } // Question type settings diff --git a/admin/settings/server.php b/admin/settings/server.php index 179cc982abb..1ae55f217d1 100644 --- a/admin/settings/server.php +++ b/admin/settings/server.php @@ -546,4 +546,76 @@ if ($hassiteconfig) { new lang_string('updatenotifybuilds_desc', 'core_admin'), 0)); $ADMIN->add('server', $temp); } + + // Web services. + $ADMIN->add('server', new admin_category('webservicesettings', new lang_string('webservices', 'webservice'))); + + // Web services > Overview. + $temp = new admin_settingpage('webservicesoverview', new lang_string('webservicesoverview', 'webservice')); + $temp->add(new admin_setting_webservicesoverview()); + $ADMIN->add('webservicesettings', $temp); + + // Web services > API documentation. + $ADMIN->add('webservicesettings', new admin_externalpage('webservicedocumentation', new lang_string('wsdocapi', 'webservice'), + "{$CFG->wwwroot}/{$CFG->admin}/webservice/documentation.php", 'moodle/site:config', false)); + + // Web services > External services. + $temp = new admin_settingpage('externalservices', new lang_string('externalservices', 'webservice')); + + $temp->add(new admin_setting_heading('manageserviceshelpexplaination', new lang_string('information', 'webservice'), + new lang_string('servicehelpexplanation', 'webservice'))); + + $temp->add(new admin_setting_manageexternalservices()); + + $ADMIN->add('webservicesettings', $temp); + + $ADMIN->add('webservicesettings', new admin_externalpage('externalservice', new lang_string('editaservice', 'webservice'), + "{$CFG->wwwroot}/{$CFG->admin}/webservice/service.php", 'moodle/site:config', true)); + + $ADMIN->add('webservicesettings', new admin_externalpage('externalservicefunctions', + new lang_string('externalservicefunctions', 'webservice'), "{$CFG->wwwroot}/{$CFG->admin}/webservice/service_functions.php", + 'moodle/site:config', true)); + + $ADMIN->add('webservicesettings', new admin_externalpage('externalserviceusers', + new lang_string('externalserviceusers', 'webservice'), "{$CFG->wwwroot}/{$CFG->admin}/webservice/service_users.php", + 'moodle/site:config', true)); + + $ADMIN->add('webservicesettings', new admin_externalpage('externalserviceusersettings', + new lang_string('serviceusersettings', 'webservice'), "{$CFG->wwwroot}/{$CFG->admin}/webservice/service_user_settings.php", + 'moodle/site:config', true)); + + // Web services > Manage protocols. + $temp = new admin_settingpage('webserviceprotocols', new lang_string('manageprotocols', 'webservice')); + $temp->add(new admin_setting_managewebserviceprotocols()); + if (empty($CFG->enablewebservices)) { + $temp->add(new admin_setting_heading('webservicesaredisabled', '', new lang_string('disabledwarning', 'webservice'))); + } + + // We cannot use $OUTPUT->doc_link() this early, we would lose the ability to set the page layout on all admin pages. + $url = new moodle_url(get_docs_url('How_to_get_a_security_key')); + $wsdoclink = html_writer::link($url, new lang_string('supplyinfo', 'webservice'), ['target' => '_blank']); + $temp->add(new admin_setting_configcheckbox('enablewsdocumentation', new lang_string('enablewsdocumentation', 'admin'), + new lang_string('configenablewsdocumentation', 'admin', $wsdoclink), false)); + + $ADMIN->add('webservicesettings', $temp); + + $plugins = core_plugin_manager::instance()->get_plugins_of_type('webservice'); + core_collator::asort_objects_by_property($plugins, 'displayname'); + foreach ($plugins as $plugin) { + /** @var \core\plugininfo\webservice $plugin */ + $plugin->load_settings($ADMIN, 'webservicesettings', $hassiteconfig); + } + + // Web services > Manage tokens. + $ADMIN->add('webservicesettings', new admin_externalpage('addwebservicetoken', new lang_string('managetokens', 'webservice'), + "{$CFG->wwwroot}/{$CFG->admin}/webservice/tokens.php", 'moodle/site:config', true)); + + $temp = new admin_settingpage('webservicetokens', new lang_string('managetokens', 'webservice')); + $temp->add(new admin_setting_managewebservicetokens()); + + if (empty($CFG->enablewebservices)) { + $temp->add(new admin_setting_heading('webservicesaredisabled', '', new lang_string('disabledwarning', 'webservice'))); + } + + $ADMIN->add('webservicesettings', $temp); } diff --git a/admin/tests/behat/manage_tokens.feature b/admin/tests/behat/manage_tokens.feature index dc1cd948a60..20fda05f54d 100644 --- a/admin/tests/behat/manage_tokens.feature +++ b/admin/tests/behat/manage_tokens.feature @@ -15,7 +15,7 @@ Feature: Manage tokens @javascript Scenario: Add & delete a token - Given I navigate to "Plugins > Web services > Manage tokens" in site administration + Given I navigate to "Server > Web services > Manage tokens" in site administration And I follow "Add" And I set the field "User" to "Joe Bloggs" And I set the field "IP restriction" to "127.0.0.1" diff --git a/admin/upgrade.txt b/admin/upgrade.txt index c3ab30f87e4..6beeb3a0f42 100644 --- a/admin/upgrade.txt +++ b/admin/upgrade.txt @@ -4,6 +4,9 @@ This files describes API changes in /admin/*. * New admin setting admin_setting_encryptedpassword allows passwords in admin settings to be encrypted (with the new \core\encryption API) so that even the admin cannot read them. +* Web services administration has been moved from Plugins into the Server category. If you have + Behat tests containing steps like `Given I navigate to "Plugins > Web services > ..."`, you will + want to replace them with `Given I navigate to "Server > Web services > ..."`. === 3.9 === diff --git a/webservice/lib.php b/webservice/lib.php index 5257640e944..4ffac8253f4 100644 --- a/webservice/lib.php +++ b/webservice/lib.php @@ -1416,7 +1416,7 @@ abstract class webservice_base_server extends webservice_server { 7. The function is called with username/password (no user token is sent) and none of the services has the function to allow the user. These settings can be found in Administration > Site administration - > Plugins > Web services > External services and Manage tokens.'); + > Server > Web services > External services and Manage tokens.'); } // we have all we need now From db723ad06655c391ebd1a15c83c361a8152f9859 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Tue, 21 Jan 2020 13:21:58 +0100 Subject: [PATCH 02/15] MDL-67748 admin: Move the web service token form to its own file --- admin/webservice/forms.php | 118 ---------------------- admin/webservice/tokens.php | 3 +- webservice/classes/token_form.php | 156 ++++++++++++++++++++++++++++++ 3 files changed, 157 insertions(+), 120 deletions(-) create mode 100644 webservice/classes/token_form.php diff --git a/admin/webservice/forms.php b/admin/webservice/forms.php index c958661e347..3060fd3fab9 100644 --- a/admin/webservice/forms.php +++ b/admin/webservice/forms.php @@ -221,121 +221,3 @@ class external_service_functions_form extends moodleform { } } - -class web_service_token_form extends moodleform { - - function definition() { - global $USER, $DB, $CFG; - - $mform = $this->_form; - $data = $this->_customdata; - - $mform->addElement('header', 'token', get_string('token', 'webservice')); - - if (empty($data->nouserselection)) { - - //check if the number of user is reasonable to be displayed in a select box - $usertotal = $DB->count_records('user', - array('deleted' => 0, 'suspended' => 0, 'confirmed' => 1)); - - if ($usertotal < 500) { - list($sort, $params) = users_order_by_sql('u'); - // User searchable selector - return users who are confirmed, not deleted, not suspended and not a guest. - $userfieldsapi = \core\user_fields::for_name(); - $sql = 'SELECT u.id' . $userfieldsapi->get_sql('u')->selects . ' - FROM {user} u - WHERE u.deleted = 0 - AND u.confirmed = 1 - AND u.suspended = 0 - AND u.id != :siteguestid - ORDER BY ' . $sort; - $params['siteguestid'] = $CFG->siteguest; - $users = $DB->get_records_sql($sql, $params); - $options = array(); - foreach ($users as $userid => $user) { - $options[$userid] = fullname($user); - } - $mform->addElement('searchableselector', 'user', get_string('user'), $options); - $mform->setType('user', PARAM_INT); - } else { - //simple text box for username or user id (if two username exists, a form error is displayed) - $mform->addElement('text', 'user', get_string('usernameorid', 'webservice')); - $mform->setType('user', PARAM_RAW_TRIMMED); - } - $mform->addRule('user', get_string('required'), 'required', null, 'client'); - } - - //service selector - $services = $DB->get_records('external_services'); - $options = array(); - $systemcontext = context_system::instance(); - foreach ($services as $serviceid => $service) { - //check that the user has the required capability - //(only for generation by the profile page) - if (empty($data->nouserselection) - || empty($service->requiredcapability) - || has_capability($service->requiredcapability, $systemcontext, $USER->id)) { - $options[$serviceid] = $service->name; - } - } - $mform->addElement('select', 'service', get_string('service', 'webservice'), $options); - $mform->addRule('service', get_string('required'), 'required', null, 'client'); - $mform->setType('service', PARAM_INT); - - $mform->addElement('text', 'iprestriction', get_string('iprestriction', 'webservice')); - $mform->setType('iprestriction', PARAM_RAW_TRIMMED); - - $mform->addElement('date_selector', 'validuntil', - get_string('validuntil', 'webservice'), array('optional' => true)); - $mform->setType('validuntil', PARAM_INT); - - $mform->addElement('hidden', 'action'); - $mform->setType('action', PARAM_ALPHANUMEXT); - - $this->add_action_buttons(true); - - $this->set_data($data); - } - - function get_data() { - global $DB; - $data = parent::get_data(); - - if (!empty($data) && !is_numeric($data->user)) { - //retrieve username - $user = $DB->get_record('user', array('username' => $data->user), 'id'); - $data->user = $user->id; - } - return $data; - } - - function validation($data, $files) { - global $DB; - - $errors = parent::validation($data, $files); - - if (is_numeric($data['user'])) { - $searchtype = 'id'; - } else { - $searchtype = 'username'; - //check the username is valid - if (clean_param($data['user'], PARAM_USERNAME) != $data['user']) { - $errors['user'] = get_string('invalidusername'); - } - } - - if (!isset($errors['user'])) { - $users = $DB->get_records('user', array($searchtype => $data['user']), '', 'id'); - - //check that the user exists in the database - if (count($users) == 0) { - $errors['user'] = get_string('usernameoridnousererror', 'webservice'); - } else if (count($users) > 1) { //can only be a username search as id are unique - $errors['user'] = get_string('usernameoridoccurenceerror', 'webservice'); - } - } - - return $errors; - } - -} diff --git a/admin/webservice/tokens.php b/admin/webservice/tokens.php index b4d246a2029..21d74855f0c 100644 --- a/admin/webservice/tokens.php +++ b/admin/webservice/tokens.php @@ -25,7 +25,6 @@ */ require_once('../../config.php'); require_once($CFG->libdir . '/adminlib.php'); -require_once($CFG->dirroot . '/' . $CFG->admin . '/webservice/forms.php'); require_once($CFG->libdir . '/externallib.php'); $action = optional_param('action', '', PARAM_ALPHANUMEXT); @@ -51,7 +50,7 @@ $webservicemanager = new webservice(); switch ($action) { case 'create': - $mform = new web_service_token_form(null, array('action' => 'create')); + $mform = new \core_webservice\token_form(null, array('action' => 'create')); $data = $mform->get_data(); if ($mform->is_cancelled()) { redirect($tokenlisturl); diff --git a/webservice/classes/token_form.php b/webservice/classes/token_form.php new file mode 100644 index 00000000000..72ca11498c0 --- /dev/null +++ b/webservice/classes/token_form.php @@ -0,0 +1,156 @@ +. + +/** + * Provides the {@see \core_webservice\token_form} class. + * + * @package core_webservice + * @category admin + * @copyright 2020 David Mudrák + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace core_webservice; + +/** + * Form to create and edit a web service token. + * + * Tokens allow users call external functions provided by selected web services. They can optionally have IP restriction + * and date validity defined. + * + * @copyright 2010 Jerome Mouneyrac + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class token_form extends \moodleform { + + /** + * Defines the form fields. + */ + public function definition() { + global $USER, $DB, $CFG; + + $mform = $this->_form; + $data = $this->_customdata; + + $mform->addElement('header', 'token', get_string('token', 'webservice')); + + if (empty($data->nouserselection)) { + + //check if the number of user is reasonable to be displayed in a select box + $usertotal = $DB->count_records('user', + array('deleted' => 0, 'suspended' => 0, 'confirmed' => 1)); + + if ($usertotal < 500) { + list($sort, $params) = users_order_by_sql('u'); + // User searchable selector - return users who are confirmed, not deleted, not suspended and not a guest. + $userfieldsapi = \core\user_fields::for_name(); + $sql = 'SELECT u.id' . $userfieldsapi->get_sql('u')->selects . ' + FROM {user} u + WHERE u.deleted = 0 + AND u.confirmed = 1 + AND u.suspended = 0 + AND u.id != :siteguestid + ORDER BY ' . $sort; + $params['siteguestid'] = $CFG->siteguest; + $users = $DB->get_records_sql($sql, $params); + $options = array(); + foreach ($users as $userid => $user) { + $options[$userid] = fullname($user); + } + $mform->addElement('searchableselector', 'user', get_string('user'), $options); + $mform->setType('user', PARAM_INT); + } else { + //simple text box for username or user id (if two username exists, a form error is displayed) + $mform->addElement('text', 'user', get_string('usernameorid', 'webservice')); + $mform->setType('user', PARAM_RAW_TRIMMED); + } + $mform->addRule('user', get_string('required'), 'required', null, 'client'); + } + + //service selector + $services = $DB->get_records('external_services'); + $options = array(); + $systemcontext = \context_system::instance(); + foreach ($services as $serviceid => $service) { + //check that the user has the required capability + //(only for generation by the profile page) + if (empty($data->nouserselection) + || empty($service->requiredcapability) + || has_capability($service->requiredcapability, $systemcontext, $USER->id)) { + $options[$serviceid] = $service->name; + } + } + $mform->addElement('select', 'service', get_string('service', 'webservice'), $options); + $mform->addRule('service', get_string('required'), 'required', null, 'client'); + $mform->setType('service', PARAM_INT); + + $mform->addElement('text', 'iprestriction', get_string('iprestriction', 'webservice')); + $mform->setType('iprestriction', PARAM_RAW_TRIMMED); + + $mform->addElement('date_selector', 'validuntil', + get_string('validuntil', 'webservice'), array('optional' => true)); + $mform->setType('validuntil', PARAM_INT); + + $mform->addElement('hidden', 'action'); + $mform->setType('action', PARAM_ALPHANUMEXT); + + $this->add_action_buttons(true); + + $this->set_data($data); + } + + function get_data() { + global $DB; + $data = parent::get_data(); + + if (!empty($data) && !is_numeric($data->user)) { + //retrieve username + $user = $DB->get_record('user', array('username' => $data->user), 'id'); + $data->user = $user->id; + } + return $data; + } + + function validation($data, $files) { + global $DB; + + $errors = parent::validation($data, $files); + + if (is_numeric($data['user'])) { + $searchtype = 'id'; + } else { + $searchtype = 'username'; + //check the username is valid + if (clean_param($data['user'], PARAM_USERNAME) != $data['user']) { + $errors['user'] = get_string('invalidusername'); + } + } + + if (!isset($errors['user'])) { + $users = $DB->get_records('user', array($searchtype => $data['user']), '', 'id'); + + //check that the user exists in the database + if (count($users) == 0) { + $errors['user'] = get_string('usernameoridnousererror', 'webservice'); + } else if (count($users) > 1) { //can only be a username search as id are unique + $errors['user'] = get_string('usernameoridoccurenceerror', 'webservice'); + } + } + + return $errors; + } + +} From 614036d79767840436da3360db2913eaa0ffd034 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Mon, 22 Feb 2021 11:08:48 +0100 Subject: [PATCH 03/15] MDL-67748 user: Fix @link docs tag usage Not related to this issue, just spotted while working on it. --- user/tests/externallib_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/user/tests/externallib_test.php b/user/tests/externallib_test.php index 2e179df56eb..0f49fdbcaa9 100644 --- a/user/tests/externallib_test.php +++ b/user/tests/externallib_test.php @@ -679,7 +679,7 @@ class core_user_externallib_testcase extends externallib_advanced_testcase { } /** - * Data provider for {@link self::test_create_users_invalid_parameter()}. + * Data provider for {@see self::test_create_users_invalid_parameter()}. * * @return array */ From f51d3a81d6132a90abd5a3eaea10ce768304cb2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Wed, 22 Jan 2020 23:00:58 +0100 Subject: [PATCH 04/15] MDL-67748 user: Add a new core_user_search_identity external function The purpose of this external function is to provide data for asynchronous user selectors and similar widgets. It allows to search users matching the given query in their name or other available identity fields. --- lib/db/services.php | 11 +- user/classes/external/search_identity.php | 131 +++++++++++++++++++ user/tests/externallib_test.php | 145 ++++++++++++++++++++++ user/upgrade.txt | 6 + version.php | 2 +- 5 files changed, 293 insertions(+), 2 deletions(-) create mode 100644 user/classes/external/search_identity.php diff --git a/lib/db/services.php b/lib/db/services.php index 75d858d6609..adec2e98912 100644 --- a/lib/db/services.php +++ b/lib/db/services.php @@ -1740,12 +1740,21 @@ $functions = array( 'methodname' => 'get_users_by_field', 'classpath' => 'user/externallib.php', 'description' => 'Retrieve users\' information for a specified unique field - If you want to do a user search, use ' - . 'core_user_get_users()', + . 'core_user_get_users() or core_user_search_identity().', 'type' => 'read', 'capabilities' => 'moodle/user:viewdetails, moodle/user:viewhiddendetails, moodle/course:useremail, moodle/user:update', 'ajax' => true, 'services' => array(MOODLE_OFFICIAL_MOBILE_SERVICE), ), + 'core_user_search_identity' => array( + 'classname' => '\core_user\external\search_identity', + 'methodname' => 'execute', + 'description' => 'Return list of users identities matching the given criteria in their name or other identity fields.', + 'type' => 'read', + 'capabilities' => 'moodle/user:viewalldetails', + 'ajax' => true, + 'loginrequired' => true, + ), 'core_user_remove_user_device' => array( 'classname' => 'core_user_external', 'methodname' => 'remove_user_device', diff --git a/user/classes/external/search_identity.php b/user/classes/external/search_identity.php new file mode 100644 index 00000000000..694f3920594 --- /dev/null +++ b/user/classes/external/search_identity.php @@ -0,0 +1,131 @@ +. + +namespace core_user\external; + +/** + * Provides the core_user_search_identity external function. + * + * @package core_user + * @category external + * @copyright 2021 David Mudrák + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class search_identity extends \external_api { + + /** + * Describes the external function parameters. + * + * @return external_function_parameters + */ + public static function execute_parameters(): \external_function_parameters { + + return new \external_function_parameters([ + 'query' => new \external_value(PARAM_TEXT, 'The search query', VALUE_REQUIRED), + ]); + } + + /** + * Finds users with the identity matching the given query. + * + * @param string $query The search request. + * @return array + */ + public static function execute(string $query): array { + global $DB, $CFG; + + $params = \external_api::validate_parameters(self::execute_parameters(), [ + 'query' => $query, + ]); + $query = $params['query']; + + // Validate context. + $context = \context_system::instance(); + self::validate_context($context); + require_capability('moodle/user:viewalldetails', $context); + + $hasviewfullnames = has_capability('moodle/site:viewfullnames', $context); + + $fields = \core\user_fields::for_name()->with_identity($context, false); + $extrafields = $fields->get_required_fields([\core\user_fields::PURPOSE_IDENTITY]); + + list($searchsql, $searchparams) = users_search_sql($query, '', true, $extrafields); + list($sortsql, $sortparams) = users_order_by_sql('', $query, $context); + $params = array_merge($searchparams, $sortparams); + + $rs = $DB->get_recordset_select('user', $searchsql, $params, $sortsql, + 'id' . $fields->get_sql()->selects, 0, $CFG->maxusersperpage + 1); + + $count = 0; + $list = []; + + foreach ($rs as $record) { + $user = (object)[ + 'id' => $record->id, + 'fullname' => fullname($record, $hasviewfullnames), + 'extrafields' => [], + ]; + + foreach ($extrafields as $extrafield) { + // Sanitize the extra fields to prevent potential XSS exploit. + $user->extrafields[] = (object)[ + 'name' => $extrafield, + 'value' => s($record->$extrafield) + ]; + } + + $count++; + + if ($count <= $CFG->maxusersperpage) { + $list[$record->id] = $user; + } + } + + $rs->close(); + + return [ + 'list' => $list, + 'maxusersperpage' => $CFG->maxusersperpage, + 'overflow' => ($count > $CFG->maxusersperpage), + ]; + } + + /** + * Describes the external function result value. + * + * @return external_description + */ + public static function execute_returns(): \external_description { + + return new \external_single_structure([ + 'list' => new \external_multiple_structure( + new \external_single_structure([ + 'id' => new \external_value(\core_user::get_property_type('id'), 'ID of the user'), + // The output of the {@see fullname()} can contain formatting HTML such as tags. + // So we need PARAM_RAW here and the caller is supposed to render it appropriately. + 'fullname' => new \external_value(PARAM_RAW, 'The fullname of the user'), + 'extrafields' => new \external_multiple_structure( + new \external_single_structure([ + 'name' => new \external_value(PARAM_TEXT, 'Name of the extrafield.'), + 'value' => new \external_value(PARAM_TEXT, 'Value of the extrafield.'), + ]), 'List of extra fields', VALUE_OPTIONAL) + ]) + ), + 'maxusersperpage' => new \external_value(PARAM_INT, 'Configured maximum users per page.'), + 'overflow' => new \external_value(PARAM_BOOL, 'Were there more records than maxusersperpage found?'), + ]); + } +} diff --git a/user/tests/externallib_test.php b/user/tests/externallib_test.php index 0f49fdbcaa9..3c6b4de806d 100644 --- a/user/tests/externallib_test.php +++ b/user/tests/externallib_test.php @@ -1538,4 +1538,149 @@ class core_user_externallib_testcase extends externallib_advanced_testcase { // Try to retrieve other user private files info. core_user_external::get_private_files_info($user2->id); } + + /** + * Test the functionality of the {@see \core_user\external\search_identity} class. + */ + public function test_external_search_identity() { + global $CFG; + + $this->resetAfterTest(true); + $this->setAdminUser(); + + $user1 = self::getDataGenerator()->create_user([ + 'firstname' => 'Firstone', + 'lastname' => 'Lastone', + 'username' => 'usernameone', + 'idnumber' => 'idnumberone', + 'email' => 'userone@example.com', + 'phone1' => 'tel1', + 'phone2' => 'tel2', + 'department' => 'Department Foo', + 'institution' => 'Institution Foo', + 'city' => 'City One', + 'country' => 'AU', + ]); + + $user2 = self::getDataGenerator()->create_user([ + 'firstname' => 'Firsttwo', + 'lastname' => 'Lasttwo', + 'username' => 'usernametwo', + 'idnumber' => 'idnumbertwo', + 'email' => 'usertwo@example.com', + 'phone1' => 'tel1', + 'phone2' => 'tel2', + 'department' => 'Department Foo', + 'institution' => 'Institution Foo', + 'city' => 'City One', + 'country' => 'AU', + ]); + + $user3 = self::getDataGenerator()->create_user([ + 'firstname' => 'Firstthree', + 'lastname' => 'Lastthree', + 'username' => 'usernamethree', + 'idnumber' => 'idnumberthree', + 'email' => 'userthree@example.com', + 'phone1' => 'tel1', + 'phone2' => 'tel2', + 'department' => 'Department Foo', + 'institution' => 'Institution Foo', + 'city' => 'City One', + 'country' => 'AU', + ]); + + $CFG->showuseridentity = 'email,idnumber,city'; + $CFG->maxusersperpage = 3; + + $result = \core_user\external\search_identity::execute('Lastt'); + $result = external_api::clean_returnvalue(\core_user\external\search_identity::execute_returns(), $result); + + $this->assertEquals(2, count($result['list'])); + $this->assertEquals(3, $result['maxusersperpage']); + $this->assertEquals(false, $result['overflow']); + + foreach ($result['list'] as $user) { + $this->assertEquals(3, count($user['extrafields'])); + $this->assertEquals('email', $user['extrafields'][0]['name']); + $this->assertEquals('idnumber', $user['extrafields'][1]['name']); + $this->assertEquals('city', $user['extrafields'][2]['name']); + } + + $CFG->showuseridentity = 'username'; + $CFG->maxusersperpage = 2; + + $result = \core_user\external\search_identity::execute('Firstt'); + $result = external_api::clean_returnvalue(\core_user\external\search_identity::execute_returns(), $result); + + $this->assertEquals(2, count($result['list'])); + $this->assertEquals(2, $result['maxusersperpage']); + $this->assertEquals(false, $result['overflow']); + + foreach ($result['list'] as $user) { + $this->assertEquals(1, count($user['extrafields'])); + $this->assertEquals('username', $user['extrafields'][0]['name']); + } + + $CFG->showuseridentity = 'email'; + $CFG->maxusersperpage = 2; + + $result = \core_user\external\search_identity::execute('City One'); + $result = external_api::clean_returnvalue(\core_user\external\search_identity::execute_returns(), $result); + + $this->assertEquals(0, count($result['list'])); + $this->assertEquals(2, $result['maxusersperpage']); + $this->assertEquals(false, $result['overflow']); + + $CFG->showuseridentity = 'city'; + $CFG->maxusersperpage = 2; + + foreach ($result['list'] as $user) { + $this->assertEquals(1, count($user['extrafields'])); + $this->assertEquals('username', $user['extrafields'][0]['name']); + } + + $result = \core_user\external\search_identity::execute('City One'); + $result = external_api::clean_returnvalue(\core_user\external\search_identity::execute_returns(), $result); + + $this->assertEquals(2, count($result['list'])); + $this->assertEquals(2, $result['maxusersperpage']); + $this->assertEquals(true, $result['overflow']); + } + + /** + * Test functionality of the {@see \core_user\external\search_identity} class with alternativefullnameformat defined. + */ + public function test_external_search_identity_with_alternativefullnameformat() { + global $CFG; + + $this->resetAfterTest(true); + $this->setAdminUser(); + + $user1 = self::getDataGenerator()->create_user([ + 'lastname' => '小柳', + 'lastnamephonetic' => 'Koyanagi', + 'firstname' => '秋', + 'firstnamephonetic' => 'Aki', + 'email' => 'koyanagiaki@example.com', + 'country' => 'JP', + ]); + + $CFG->showuseridentity = 'email'; + $CFG->maxusersperpage = 3; + $CFG->alternativefullnameformat = + 'lastname firstname (lastnamephonetic firstnamephonetic)'; + + $result = \core_user\external\search_identity::execute('Ak'); + $result = external_api::clean_returnvalue(\core_user\external\search_identity::execute_returns(), $result); + + $this->assertEquals(1, count($result['list'])); + $this->assertEquals(3, $result['maxusersperpage']); + $this->assertEquals(false, $result['overflow']); + + foreach ($result['list'] as $user) { + $this->assertEquals(1, count($user['extrafields'])); + $this->assertEquals('email', $user['extrafields'][0]['name']); + } + } } diff --git a/user/upgrade.txt b/user/upgrade.txt index 85531920a8d..3d4c3624782 100644 --- a/user/upgrade.txt +++ b/user/upgrade.txt @@ -1,5 +1,11 @@ This files describes API changes for code that uses the user API. +=== 3.11 === + +* Added new external function core_user_external::search_identity(). The main purpose of this external function is to + provide data for asynchronous user selectors and similar widgets. It allows to search users matching the given query + in their name or other available identity fields. + === 3.9 === * The unified filter has been replaced by the participants filter. The following have therefore been deprecated: diff --git a/version.php b/version.php index 56afbad5b80..8b558c60aa7 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2021052500.68; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2021052500.69; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes. $release = '4.0dev (Build: 20210312)'; // Human-friendly version name From b389393dc58e99df3bb7b4c8ddc1bc2e1c424c5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Wed, 22 Jan 2020 23:18:11 +0100 Subject: [PATCH 05/15] MDL-67748 user: Add new core_user/form_user_selector JS module The module provides AJAX support for the autocomplete form element. --- user/amd/build/form_user_selector.min.js | 2 + user/amd/build/form_user_selector.min.js.map | 1 + user/amd/src/form_user_selector.js | 87 +++++++++++++++++++ .../form_user_selector_suggestion.mustache | 54 ++++++++++++ user/upgrade.txt | 2 + 5 files changed, 146 insertions(+) create mode 100644 user/amd/build/form_user_selector.min.js create mode 100644 user/amd/build/form_user_selector.min.js.map create mode 100644 user/amd/src/form_user_selector.js create mode 100644 user/templates/form_user_selector_suggestion.mustache diff --git a/user/amd/build/form_user_selector.min.js b/user/amd/build/form_user_selector.min.js new file mode 100644 index 00000000000..41c53a6abc6 --- /dev/null +++ b/user/amd/build/form_user_selector.min.js @@ -0,0 +1,2 @@ +define ("core_user/form_user_selector",["exports","core/ajax","core/templates","core/str"],function(a,b,c,d){"use strict";Object.defineProperty(a,"__esModule",{value:!0});a.transport=g;a.processResults=function(a,b){if(!Array.isArray(b)){return b}else{return b.map(function(a){return{value:a.id,label:a.label}})}};b=function(a){return a&&a.__esModule?a:{default:a}}(b);function e(a,b,c,d,e,f,g){try{var h=a[f](g),i=h.value}catch(a){c(a);return}if(h.done){b(i)}else{Promise.resolve(i).then(d,e)}}function f(a){return function(){var b=this,c=arguments;return new Promise(function(d,f){var i=a.apply(b,c);function g(a){e(i,d,f,g,h,"next",a)}function h(a){e(i,d,f,g,h,"throw",a)}g(void 0)})}}function g(){return h.apply(this,arguments)}function h(){h=f(regeneratorRuntime.mark(function a(e,f,g,h){var i,j,k,l;return regeneratorRuntime.wrap(function(a){while(1){switch(a.prev=a.next){case 0:i={methodname:"core_user_search_identity",args:{query:f}};a.prev=1;a.next=4;return b.default.call([i])[0];case 4:j=a.sent;if(!j.overflow){a.next=12;break}a.next=8;return(0,d.get_string)("toomanyuserstoshow","core",">"+j.maxusersperpage);case 8:k=a.sent;g(k);a.next=19;break;case 12:l=[];j.list.forEach(function(a){l.push((0,c.render)("core_user/form_user_selector_suggestion",a))});a.next=16;return Promise.all(l);case 16:l=a.sent;j.list.forEach(function(a,b){a.label=l[b]});g(j.list);case 19:a.next=24;break;case 21:a.prev=21;a.t0=a["catch"](1);h(a.t0);case 24:case"end":return a.stop();}}},a,null,[[1,21]])}));return h.apply(this,arguments)}}); +//# sourceMappingURL=form_user_selector.min.js.map diff --git a/user/amd/build/form_user_selector.min.js.map b/user/amd/build/form_user_selector.min.js.map new file mode 100644 index 00000000000..0b7ebcb9a1a --- /dev/null +++ b/user/amd/build/form_user_selector.min.js.map @@ -0,0 +1 @@ +{"version":3,"sources":["../src/form_user_selector.js"],"names":["selector","results","Array","isArray","map","result","value","id","label","transport","query","callback","failure","request","methodname","args","Ajax","call","response","overflow","maxusersperpage","msg","labels","list","forEach","user","push","Promise","all","index"],"mappings":"0MA8EO,SAAwBA,CAAxB,CAAkCC,CAAlC,CAA2C,CAE9C,GAAI,CAACC,KAAK,CAACC,OAAN,CAAcF,CAAd,CAAL,CAA6B,CACzB,MAAOA,CAAAA,CAEV,CAHD,IAGO,CACH,MAAOA,CAAAA,CAAO,CAACG,GAAR,CAAY,SAAAC,CAAM,QAAK,CAACC,KAAK,CAAED,CAAM,CAACE,EAAf,CAAmBC,KAAK,CAAEH,CAAM,CAACG,KAAjC,CAAL,CAAlB,CACV,CACJ,C,CA9DD,uD,uUAYsBC,CAAAA,C,2EAAf,WAAyBT,CAAzB,CAAmCU,CAAnC,CAA0CC,CAA1C,CAAoDC,CAApD,+FAEGC,CAFH,CAEa,CACZC,UAAU,CAAE,2BADA,CAEZC,IAAI,CAAE,CACFL,KAAK,CAAEA,CADL,CAFM,CAFb,yBAUwBM,WAAKC,IAAL,CAAU,CAACJ,CAAD,CAAV,EAAqB,CAArB,CAVxB,QAUOK,CAVP,YAYKA,CAAQ,CAACC,QAZd,iCAauB,iBAAU,oBAAV,CAAgC,MAAhC,CAAwC,IAAMD,CAAQ,CAACE,eAAvD,CAbvB,QAaWC,CAbX,QAcKV,CAAQ,CAACU,CAAD,CAAR,CAdL,wBAiBSC,CAjBT,CAiBkB,EAjBlB,CAkBKJ,CAAQ,CAACK,IAAT,CAAcC,OAAd,CAAsB,SAAAC,CAAI,CAAI,CAC1BH,CAAM,CAACI,IAAP,CAAY,aAAe,yCAAf,CAA0DD,CAA1D,CAAZ,CACH,CAFD,EAlBL,gBAqBoBE,CAAAA,OAAO,CAACC,GAAR,CAAYN,CAAZ,CArBpB,SAqBKA,CArBL,QAuBKJ,CAAQ,CAACK,IAAT,CAAcC,OAAd,CAAsB,SAACC,CAAD,CAAOI,CAAP,CAAiB,CACnCJ,CAAI,CAACjB,KAAL,CAAac,CAAM,CAACO,CAAD,CACtB,CAFD,EAIAlB,CAAQ,CAACO,CAAQ,CAACK,IAAV,CAAR,CA3BL,6DA+BCX,CAAO,MAAP,CA/BD,uD","sourcesContent":["// This file is part of Moodle - https://moodle.org/\n//\n// Moodle is free software: you can redistribute it and/or modify\n// it under the terms of the GNU General Public License as published by\n// the Free Software Foundation, either version 3 of the License, or\n// (at your option) any later version.\n//\n// Moodle is distributed in the hope that it will be useful,\n// but WITHOUT ANY WARRANTY; without even the implied warranty of\n// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the\n// GNU General Public License for more details.\n//\n// You should have received a copy of the GNU General Public License\n// along with Moodle. If not, see .\n\n/**\n * Provides the required functionality for an autocomplete element to select a user.\n *\n * @module core_user/form_user_selector\n * @package core_webservice\n * @copyright 2020 David Mudrák \n * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later\n */\n\nimport Ajax from 'core/ajax';\nimport {render as renderTemplate} from 'core/templates';\nimport {get_string as getString} from 'core/str';\n\n/**\n * Load the list of users matching the query and render the selector labels for them.\n *\n * @param {String} selector The selector of the auto complete element.\n * @param {String} query The query string.\n * @param {Function} callback A callback function receiving an array of results.\n * @param {Function} failure A function to call in case of failure, receiving the error message.\n */\nexport async function transport(selector, query, callback, failure) {\n\n const request = {\n methodname: 'core_user_search_identity',\n args: {\n query: query\n }\n };\n\n try {\n const response = await Ajax.call([request])[0];\n\n if (response.overflow) {\n const msg = await getString('toomanyuserstoshow', 'core', '>' + response.maxusersperpage);\n callback(msg);\n\n } else {\n let labels = [];\n response.list.forEach(user => {\n labels.push(renderTemplate('core_user/form_user_selector_suggestion', user));\n });\n labels = await Promise.all(labels);\n\n response.list.forEach((user, index) => {\n user.label = labels[index];\n });\n\n callback(response.list);\n }\n\n } catch (e) {\n failure(e);\n }\n}\n\n/**\n * Process the results for auto complete elements.\n *\n * @param {String} selector The selector of the auto complete element.\n * @param {Array} results An array or results returned by {@see transport()}.\n * @return {Array} New array of the selector options.\n */\nexport function processResults(selector, results) {\n\n if (!Array.isArray(results)) {\n return results;\n\n } else {\n return results.map(result => ({value: result.id, label: result.label}));\n }\n}\n"],"file":"form_user_selector.min.js"} \ No newline at end of file diff --git a/user/amd/src/form_user_selector.js b/user/amd/src/form_user_selector.js new file mode 100644 index 00000000000..07df01c03be --- /dev/null +++ b/user/amd/src/form_user_selector.js @@ -0,0 +1,87 @@ +// This file is part of Moodle - https://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 . + +/** + * Provides the required functionality for an autocomplete element to select a user. + * + * @module core_user/form_user_selector + * @package core_webservice + * @copyright 2020 David Mudrák + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +import Ajax from 'core/ajax'; +import {render as renderTemplate} from 'core/templates'; +import {get_string as getString} from 'core/str'; + +/** + * Load the list of users matching the query and render the selector labels for them. + * + * @param {String} selector The selector of the auto complete element. + * @param {String} query The query string. + * @param {Function} callback A callback function receiving an array of results. + * @param {Function} failure A function to call in case of failure, receiving the error message. + */ +export async function transport(selector, query, callback, failure) { + + const request = { + methodname: 'core_user_search_identity', + args: { + query: query + } + }; + + try { + const response = await Ajax.call([request])[0]; + + if (response.overflow) { + const msg = await getString('toomanyuserstoshow', 'core', '>' + response.maxusersperpage); + callback(msg); + + } else { + let labels = []; + response.list.forEach(user => { + labels.push(renderTemplate('core_user/form_user_selector_suggestion', user)); + }); + labels = await Promise.all(labels); + + response.list.forEach((user, index) => { + user.label = labels[index]; + }); + + callback(response.list); + } + + } catch (e) { + failure(e); + } +} + +/** + * Process the results for auto complete elements. + * + * @param {String} selector The selector of the auto complete element. + * @param {Array} results An array or results returned by {@see transport()}. + * @return {Array} New array of the selector options. + */ +export function processResults(selector, results) { + + if (!Array.isArray(results)) { + return results; + + } else { + return results.map(result => ({value: result.id, label: result.label})); + } +} diff --git a/user/templates/form_user_selector_suggestion.mustache b/user/templates/form_user_selector_suggestion.mustache new file mode 100644 index 00000000000..abec063a0a7 --- /dev/null +++ b/user/templates/form_user_selector_suggestion.mustache @@ -0,0 +1,54 @@ +{{! + This file is part of Moodle - https://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 . +}} +{{! + @template core_user/form_user_selector_suggestion + + Moodle template for the list of valid options in an user selector autocomplate form element. + + Classes required for JS: + * none + + Data attributes required for JS: + * none + + Context variables required for this template: + * fullname - string Users full name + * extrafields - list + + Example context (json): + { + "fullname": "Admin User", + "extrafields": [ + { + "name": "email", + "value": "admin@example.com" + }, + { + "name": "phone1", + "value": "0123456789" + } + ] + } +}} + + {{fullname}} + + {{#extrafields}} + {{{value}}} + {{/extrafields}} + + diff --git a/user/upgrade.txt b/user/upgrade.txt index 3d4c3624782..d20af1afffd 100644 --- a/user/upgrade.txt +++ b/user/upgrade.txt @@ -2,6 +2,8 @@ This files describes API changes for code that uses the user API. === 3.11 === +* Added new core_user/form_user_selector JS module that can be used as the 'ajax' handler for the autocomplete form + element implementing the user selector. * Added new external function core_user_external::search_identity(). The main purpose of this external function is to provide data for asynchronous user selectors and similar widgets. It allows to search users matching the given query in their name or other available identity fields. From a7f693429722f70c3e32096670209b78874aa7fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Wed, 22 Jan 2020 23:57:06 +0100 Subject: [PATCH 06/15] MDL-67748 admin: Respect the viewfullnames cap at manage tokens table --- webservice/classes/token_table.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/webservice/classes/token_table.php b/webservice/classes/token_table.php index f6220570031..3f2e0153884 100644 --- a/webservice/classes/token_table.php +++ b/webservice/classes/token_table.php @@ -44,6 +44,9 @@ class token_table extends \table_sql { */ protected $showalltokens; + /** @var bool $hasviewfullnames Does the user have the viewfullnames capability. */ + protected $hasviewfullnames; + /** * Sets up the table. * @param int $id The id of the table @@ -56,6 +59,7 @@ class token_table extends \table_sql { // Can we see tokens created by all users? $this->showalltokens = has_capability('moodle/webservice:managealltokens', $context); + $this->hasviewfullnames = has_capability('moodle/site:viewfullnames', $context); // Define the headers and columns. $headers = []; @@ -131,7 +135,7 @@ class token_table extends \table_sql { global $OUTPUT; $userprofilurl = new \moodle_url('/user/profile.php', ['id' => $data->userid]); - $content = \html_writer::link($userprofilurl, fullname($data)); + $content = \html_writer::link($userprofilurl, fullname($data, $this->hasviewfullnames)); // Make up list of capabilities that the user is missing for the given webservice. $webservicemanager = new \webservice(); @@ -183,7 +187,7 @@ class token_table extends \table_sql { } $creatorprofileurl = new \moodle_url('/user/profile.php', ['id' => $data->creatorid]); - return \html_writer::link($creatorprofileurl, fullname((object)$user)); + return \html_writer::link($creatorprofileurl, fullname((object)$user, $this->hasviewfullnames)); } /** From 0bcaab32ad7b6fd15953865387b1b99984a72624 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Thu, 23 Jan 2020 08:46:54 +0100 Subject: [PATCH 07/15] MDL-67748 admin: Simplify the webservice token form We can now use the autocomplete element to select the user. No need to for the legacy behaviour. --- lang/en/deprecated.txt | 2 + lang/en/webservice.php | 6 +- webservice/classes/token_form.php | 115 ++++++++++-------------------- 3 files changed, 45 insertions(+), 78 deletions(-) diff --git a/lang/en/deprecated.txt b/lang/en/deprecated.txt index 415402af298..736fb8809aa 100644 --- a/lang/en/deprecated.txt +++ b/lang/en/deprecated.txt @@ -129,3 +129,5 @@ hidepicture,core_group hidepicture,core_moodle sitebackpack,core_badges sitebackpack_help,core_badges +usernameoridnousererror,core_webservice +usernameoridoccurenceerror,core_webservice diff --git a/lang/en/webservice.php b/lang/en/webservice.php index eef36b1e061..cdf7d0a87ab 100644 --- a/lang/en/webservice.php +++ b/lang/en/webservice.php @@ -215,8 +215,6 @@ $string['userasclientsdescription'] = 'The following steps help you to set up th $string['usermissingcaps'] = 'Missing capabilities: {$a}'; $string['usernameorid'] = 'Username / User id'; $string['usernameorid_help'] = 'Enter a username or a user id.'; -$string['usernameoridnousererror'] = 'No users were found with this username/user id.'; -$string['usernameoridoccurenceerror'] = 'More than one user was found with this username. Please enter the user id.'; $string['usernotallowed'] = 'The user is not allowed for this service. First you need to allow this user on the {$a}\'s allowed users administration page.'; $string['userservices'] = 'User services: {$a}'; $string['usersettingssaved'] = 'User settings saved'; @@ -243,3 +241,7 @@ $string['wsusername'] = 'Web service username'; // Deprecated since Moodle 3.9. $string['documentation'] = 'web service documentation'; + +// Deprecated since Moodle 3.11. +$string['usernameoridnousererror'] = 'No users were found with this username/user id.'; +$string['usernameoridoccurenceerror'] = 'More than one user was found with this username. Please enter the user id.'; diff --git a/webservice/classes/token_form.php b/webservice/classes/token_form.php index 72ca11498c0..5331e96db05 100644 --- a/webservice/classes/token_form.php +++ b/webservice/classes/token_form.php @@ -40,59 +40,45 @@ class token_form extends \moodleform { * Defines the form fields. */ public function definition() { - global $USER, $DB, $CFG; + global $DB; $mform = $this->_form; $data = $this->_customdata; $mform->addElement('header', 'token', get_string('token', 'webservice')); - if (empty($data->nouserselection)) { + // User selector. + $attributes = [ + 'multiple' => false, + 'ajax' => 'core_user/form_user_selector', + 'valuehtmlcallback' => function($userid) { + global $DB, $OUTPUT; - //check if the number of user is reasonable to be displayed in a select box - $usertotal = $DB->count_records('user', - array('deleted' => 0, 'suspended' => 0, 'confirmed' => 1)); + $context = \context_system::instance(); + $fields = \core\user_fields::for_name()->with_identity($context, false); + $record = $DB->get_record('user', ['id' => $userid], $fields, MUST_EXIST); - if ($usertotal < 500) { - list($sort, $params) = users_order_by_sql('u'); - // User searchable selector - return users who are confirmed, not deleted, not suspended and not a guest. - $userfieldsapi = \core\user_fields::for_name(); - $sql = 'SELECT u.id' . $userfieldsapi->get_sql('u')->selects . ' - FROM {user} u - WHERE u.deleted = 0 - AND u.confirmed = 1 - AND u.suspended = 0 - AND u.id != :siteguestid - ORDER BY ' . $sort; - $params['siteguestid'] = $CFG->siteguest; - $users = $DB->get_records_sql($sql, $params); - $options = array(); - foreach ($users as $userid => $user) { - $options[$userid] = fullname($user); + $user = (object)[ + 'id' => $record->id, + 'fullname' => fullname($record, has_capability('moodle/site:viewfullnames', $context)), + 'extrafields' => [], + ]; + + foreach ($fields->get_required_fields([\core\user_fields::PURPOSE_IDENTITY]) as $extrafield) { + $user->extrafields[] = (object)[ + 'name' => $extrafield, + 'value' => s($record->$extrafield) + ]; } - $mform->addElement('searchableselector', 'user', get_string('user'), $options); - $mform->setType('user', PARAM_INT); - } else { - //simple text box for username or user id (if two username exists, a form error is displayed) - $mform->addElement('text', 'user', get_string('usernameorid', 'webservice')); - $mform->setType('user', PARAM_RAW_TRIMMED); - } - $mform->addRule('user', get_string('required'), 'required', null, 'client'); - } - //service selector - $services = $DB->get_records('external_services'); - $options = array(); - $systemcontext = \context_system::instance(); - foreach ($services as $serviceid => $service) { - //check that the user has the required capability - //(only for generation by the profile page) - if (empty($data->nouserselection) - || empty($service->requiredcapability) - || has_capability($service->requiredcapability, $systemcontext, $USER->id)) { - $options[$serviceid] = $service->name; - } - } + return $OUTPUT->render_from_template('core_user/form_user_selector_suggestion', $user); + }, + ]; + $mform->addElement('autocomplete', 'user', get_string('user'), [], $attributes); + $mform->addRule('user', get_string('required'), 'required', null, 'client'); + + // Service selector. + $options = $DB->get_records_menu('external_services', null, '', 'id, name'); $mform->addElement('select', 'service', get_string('service', 'webservice'), $options); $mform->addRule('service', get_string('required'), 'required', null, 'client'); $mform->setType('service', PARAM_INT); @@ -112,45 +98,22 @@ class token_form extends \moodleform { $this->set_data($data); } - function get_data() { - global $DB; - $data = parent::get_data(); - - if (!empty($data) && !is_numeric($data->user)) { - //retrieve username - $user = $DB->get_record('user', array('username' => $data->user), 'id'); - $data->user = $user->id; - } - return $data; - } - - function validation($data, $files) { + /** + * Validate the submitted data. + * + * @param array $data Submitted data. + * @param array $files Submitted files. + * @return array Validation errors. + */ + public function validation($data, $files) { global $DB; $errors = parent::validation($data, $files); - if (is_numeric($data['user'])) { - $searchtype = 'id'; - } else { - $searchtype = 'username'; - //check the username is valid - if (clean_param($data['user'], PARAM_USERNAME) != $data['user']) { - $errors['user'] = get_string('invalidusername'); - } - } - - if (!isset($errors['user'])) { - $users = $DB->get_records('user', array($searchtype => $data['user']), '', 'id'); - - //check that the user exists in the database - if (count($users) == 0) { - $errors['user'] = get_string('usernameoridnousererror', 'webservice'); - } else if (count($users) > 1) { //can only be a username search as id are unique - $errors['user'] = get_string('usernameoridoccurenceerror', 'webservice'); - } + if ($DB->get_field('user', 'suspended', ['id' => $data['user']], MUST_EXIST)) { + $errors['user'] = get_string('suspended', 'core') . ' - ' . get_string('forbiddenwsuser', 'core_webservice'); } return $errors; } - } From c07f15d791a5a55086cc38869eb2840732167d0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Thu, 23 Jan 2020 22:49:23 +0100 Subject: [PATCH 08/15] MDL-67748 admin: Improve get_missing_capabilities_by_users() The previous implementation falsely reported all implicit capabilities inherited from the authenticated user archetype. That caused a lot of capabilities reported as missing, even if they were correctly granted. This new implementation uses a different logic. Instead of seeking for explicitly assigned capabilities, it searches for capabilities that are not assigned to any of the user's role across the system. Please refer to the inline documentation. This should be still used for informative reports only, not for actual permissions evaluation. The context has been ignored here, as well as all the overrides etc. This patch just makes it a lesser evil. --- webservice/lib.php | 117 +++++++++++++++++++++++++--------- webservice/tests/lib_test.php | 59 +++++++++++++++++ webservice/upgrade.txt | 6 ++ 3 files changed, 152 insertions(+), 30 deletions(-) diff --git a/webservice/lib.php b/webservice/lib.php index 4ffac8253f4..4917030ecc7 100644 --- a/webservice/lib.php +++ b/webservice/lib.php @@ -623,11 +623,16 @@ class webservice { * as the front end does not display it itself. In pratice, * admins would like the info, for more info you can follow: MDL-29962 * + * @deprecated since Moodle 3.11 in MDL-67748 without a replacement. + * @todo MDL-70187 Please delete this method completely in Moodle 4.3, thank you. * @param int $userid user id * @return array */ public function get_user_capabilities($userid) { global $DB; + + debugging('webservice::get_user_capabilities() has been deprecated.', DEBUG_DEVELOPER); + //retrieve the user capabilities $sql = "SELECT DISTINCT rc.id, rc.capability FROM {role_capabilities} rc, {role_assignments} ra WHERE rc.roleid=ra.roleid AND ra.userid= ? AND rc.permission = ?"; @@ -640,45 +645,97 @@ class webservice { } /** - * Get missing user capabilities for a given service - * WARNING: do not use this "broken" function. It was created in the goal to display some capabilities - * required by users. In theory we should not need to display this kind of information - * as the front end does not display it itself. In pratice, - * admins would like the info, for more info you can follow: MDL-29962 + * Get missing user capabilities for the given service's functions. * - * @param array $users users - * @param int $serviceid service id - * @return array of missing capabilities, keys being the user ids + * Every external function can declare some required capabilities to allow for easier setup of the web services. + * However, that is supposed to be used for informational admin report only. There is no automatic evaluation of + * the declared capabilities and the context of the capability evaluation is ignored. Also, actual capability + * evaluation is much more complex as it allows for overrides etc. + * + * Returned are capabilities that the given users do not seem to have assigned anywhere at the site and that should + * be checked by the admin. + * + * Do not use this method for anything else, particularly not for any security related checks. See MDL-29962 for the + * background of why we have this - there are arguments for dropping this feature completely. + * + * @param array $users List of users to check, consisting of objects, arrays or integer ids. + * @param int $serviceid The id of the external service to check. + * @return array List of missing capabilities: (int)userid => array of (string)capabilitynames */ - public function get_missing_capabilities_by_users($users, $serviceid) { + public function get_missing_capabilities_by_users(array $users, int $serviceid): array { global $DB; - $usersmissingcaps = array(); - //retrieve capabilities required by the service - $servicecaps = $this->get_service_required_capabilities($serviceid); + // The following are default capabilities for all authenticated users and we will assume them granted. + $commoncaps = get_default_capabilities('user'); - //retrieve users missing capabilities - foreach ($users as $user) { - //cast user array into object to be a bit more flexible - if (is_array($user)) { - $user = (object) $user; - } - $usercaps = $this->get_user_capabilities($user->id); - - //detect the missing capabilities - foreach ($servicecaps as $functioname => $functioncaps) { - foreach ($functioncaps as $functioncap) { - if (!array_key_exists($functioncap, $usercaps)) { - if (!isset($usersmissingcaps[$user->id]) - or array_search($functioncap, $usersmissingcaps[$user->id]) === false) { - $usersmissingcaps[$user->id][] = $functioncap; - } - } + // Get the list of additional capabilities required by the service. + $servicecaps = []; + foreach ($this->get_service_required_capabilities($serviceid) as $service => $caps) { + foreach ($caps as $cap) { + if (empty($commoncaps[$cap])) { + $servicecaps[$cap] = true; } } } - return $usersmissingcaps; + if (empty($servicecaps)) { + return []; + } + + // Prepare a list of user ids we want to check. + $userids = []; + foreach ($users as $user) { + if (is_object($user) && isset($user->id)) { + $userids[$user->id] = true; + } else if (is_array($user) && isset($user['id'])) { + $userids[$user['id']] = true; + } else { + throw new coding_exception('Unexpected format of users list in webservice::get_missing_capabilities_by_users().'); + } + } + + // Prepare a matrix of missing capabilities x users - consider them all missing by default. + foreach (array_keys($userids) as $userid) { + foreach (array_keys($servicecaps) as $capname) { + $matrix[$userid][$capname] = true; + } + } + + list($capsql, $capparams) = $DB->get_in_or_equal(array_keys($servicecaps), SQL_PARAMS_NAMED, 'paramcap'); + list($usersql, $userparams) = $DB->get_in_or_equal(array_keys($userids), SQL_PARAMS_NAMED, 'paramuser'); + + $sql = "SELECT c.name AS capability, u.id AS userid + FROM {capabilities} c + JOIN {role_capabilities} rc ON c.name = rc.capability + JOIN {role_assignments} ra ON ra.roleid = rc.roleid + JOIN {user} u ON ra.userid = u.id + WHERE rc.permission = :capallow + AND c.name {$capsql} + AND u.id {$usersql}"; + + $params = $capparams + $userparams + [ + 'capallow' => CAP_ALLOW, + ]; + + $rs = $DB->get_recordset_sql($sql, $params); + + foreach ($rs as $record) { + // If there was a potential role assignment found that might grant the user the given capability, + // remove it from the matrix. Again, we ignore all the contexts, prohibits, prevents and other details + // of the permissions evaluations. See the function docblock for details. + unset($matrix[$record->userid][$record->capability]); + } + + $rs->close(); + + foreach ($matrix as $userid => $caps) { + $matrix[$userid] = array_keys($caps); + if (empty($matrix[$userid])) { + unset($matrix[$userid]); + } + } + + return $matrix; } /** diff --git a/webservice/tests/lib_test.php b/webservice/tests/lib_test.php index 407556b0dda..97784590e69 100644 --- a/webservice/tests/lib_test.php +++ b/webservice/tests/lib_test.php @@ -193,6 +193,65 @@ class webservice_test extends advanced_testcase { $this->assertEquals($before + 60, $token->lastaccess); } + /** + * Tests for the {@see webservice::get_missing_capabilities_by_users()} implementation. + */ + public function test_get_missing_capabilities_by_users() { + global $DB; + + $this->resetAfterTest(true); + $wsman = new webservice(); + + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(); + $user3 = $this->getDataGenerator()->create_user(); + + // Add a test web service. + $serviceid = $wsman->add_external_service((object)[ + 'name' => 'Test web service', + 'enabled' => 1, + 'requiredcapability' => '', + 'restrictedusers' => false, + 'component' => 'moodle', + 'downloadfiles' => false, + 'uploadfiles' => false, + ]); + + // Add a function to the service that does not declare any capability as required. + $wsman->add_external_function_to_service('core_webservice_get_site_info', $serviceid); + + // Users can be provided as an array of objects, arrays or integers (ids). + $this->assertEmpty($wsman->get_missing_capabilities_by_users([$user1, array($user2), $user3->id], $serviceid)); + + // Add a function to the service that declares some capability as required, but that capability is common for + // any user. Here we use 'core_message_delete_conversation' which declares 'moodle/site:deleteownmessage' which + // in turn is granted to the authenticated user archetype by default. + $wsman->add_external_function_to_service('core_message_delete_conversation', $serviceid); + + // So all three users should have this capability implicitly. + $this->assertEmpty($wsman->get_missing_capabilities_by_users([$user1, $user2, $user3], $serviceid)); + + // Add a function to the service that declares some non-common capability. Here we use + // 'core_group_add_group_members' that wants 'moodle/course:managegroups'. + $wsman->add_external_function_to_service('core_group_add_group_members', $serviceid); + + // Make it so that the $user1 has the capability in some course. + $course1 = $this->getDataGenerator()->create_course(); + $this->getDataGenerator()->enrol_user($user1->id, $course1->id, 'editingteacher'); + + // Check that no missing capability is reported for the $user1. We don't care at what actual context the + // external function call will evaluate the permission. We just check that there is a chance that the user has + // the capability somewhere. + $this->assertEmpty($wsman->get_missing_capabilities_by_users([$user1], $serviceid)); + + // But there is no place at the site where the capability would be granted to the other two users, so it is + // reported as missing. + $missing = $wsman->get_missing_capabilities_by_users([$user1, $user2, $user3], $serviceid); + $this->assertArrayNotHasKey($user1->id, $missing); + $this->assertContains('moodle/course:managegroups', $missing[$user2->id]); + $this->assertContains('moodle/course:managegroups', $missing[$user3->id]); + } + /** * Utility method that tests the parameter type of a method info's input/output parameter. * diff --git a/webservice/upgrade.txt b/webservice/upgrade.txt index ec06f9f00ad..2463b78b83d 100644 --- a/webservice/upgrade.txt +++ b/webservice/upgrade.txt @@ -3,6 +3,12 @@ information provided here is intended especially for developers. This information is intended for authors of webservices, not people writing webservice clients. +=== 3.11 === + +* The method webservice::get_user_capabilities() is deprecated now without a replacement. It has been used + internally only to populate the list of missing capabilities. That functionality has been improved so that + it no longer needs this standalone method. + === 3.10 === * The class externallib_advanced_testcase, used in unit tests, has a new function called "configure_filters" to easily configure filters for external functions testing. From 596b0bb23e67e220541469318aec0fec105c649a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Thu, 16 Apr 2020 16:20:19 +0200 Subject: [PATCH 09/15] MDL-67748 admin: Improve the display of missing capabilities The patch improves the display of the missing capabilities on the Manage tokens page and the Select authorised users page. The list of capabilities is rendered in a collapsible area and does not occupy too much space by default. Additionally on the later page, the patch improves the display of the list of existing authorised users so that it respects the configured identity fields. --- admin/webservice/service_users.php | 2 +- webservice/classes/token_table.php | 14 ++++---- webservice/lib.php | 12 +++++-- webservice/renderer.php | 52 ++++++++++++++++++++---------- 4 files changed, 54 insertions(+), 26 deletions(-) diff --git a/admin/webservice/service_users.php b/admin/webservice/service_users.php index 907a4cc9c1f..3bcc8bcb641 100644 --- a/admin/webservice/service_users.php +++ b/admin/webservice/service_users.php @@ -108,7 +108,7 @@ $usersmissingcaps = $webservicemanager->get_missing_capabilities_by_users($allow //add the missing capabilities to the allowed users object to be displayed by renderer foreach ($allowedusers as &$alloweduser) { if (!is_siteadmin($alloweduser->id) and array_key_exists($alloweduser->id, $usersmissingcaps)) { - $alloweduser->missingcapabilities = implode(', ', $usersmissingcaps[$alloweduser->id]); + $alloweduser->missingcapabilities = $usersmissingcaps[$alloweduser->id]; } } diff --git a/webservice/classes/token_table.php b/webservice/classes/token_table.php index 3f2e0153884..8e4e6bdf131 100644 --- a/webservice/classes/token_table.php +++ b/webservice/classes/token_table.php @@ -142,12 +142,14 @@ class token_table extends \table_sql { $usermissingcaps = $webservicemanager->get_missing_capabilities_by_users([['id' => $data->userid]], $data->serviceid); if (!is_siteadmin($data->userid) && array_key_exists($data->userid, $usermissingcaps)) { - $missingcapabilities = implode(', ', $usermissingcaps[$data->userid]); - if (!empty($missingcapabilities)) { - $capabilitiesstring = get_string('usermissingcaps', 'webservice', $missingcapabilities) . ' ' . - $OUTPUT->help_icon('missingcaps', 'webservice'); - $content .= \html_writer::div($capabilitiesstring, 'missingcaps'); - } + $count = \html_writer::span(count($usermissingcaps[$data->userid]), 'badge badge-danger'); + $links = array_map(function($capname) { + return get_capability_docs_link((object)['name' => $capname]) . \html_writer::div($capname, 'text-muted'); + }, $usermissingcaps[$data->userid]); + $list = \html_writer::alist($links); + $help = $OUTPUT->help_icon('missingcaps', 'webservice'); + $content .= print_collapsible_region(\html_writer::div($list . $help, 'missingcaps'), 'small', + \html_writer::random_id('usermissingcaps'), get_string('usermissingcaps', 'webservice', $count), '', true, true); } return $content; diff --git a/webservice/lib.php b/webservice/lib.php index 4917030ecc7..d8bd2cd34f6 100644 --- a/webservice/lib.php +++ b/webservice/lib.php @@ -285,9 +285,16 @@ class webservice { */ public function get_ws_authorised_users($serviceid) { global $DB, $CFG; + $params = array($CFG->siteguest, $serviceid); - $sql = " SELECT u.id as id, esu.id as serviceuserid, u.email as email, u.firstname as firstname, - u.lastname as lastname, + + $namefields = get_all_user_name_fields(true, 'u'); + + foreach (get_extra_user_fields(context_system::instance()) as $extrafield) { + $namefields .= ',u.' . $extrafield; + } + + $sql = " SELECT u.id as id, esu.id as serviceuserid, {$namefields}, esu.iprestriction as iprestriction, esu.validuntil as validuntil, esu.timecreated as timecreated FROM {user} u, {external_services_users} esu @@ -296,6 +303,7 @@ class webservice { AND esu.externalserviceid = ?"; $users = $DB->get_records_sql($sql, $params); + return $users; } diff --git a/webservice/renderer.php b/webservice/renderer.php index f97c03c866d..1f8a5e6a592 100644 --- a/webservice/renderer.php +++ b/webservice/renderer.php @@ -96,7 +96,7 @@ class core_webservice_renderer extends plugin_renderer_base { } /** - * Display list of authorised users + * Display list of authorised users for the given external service. * * @param array $users authorised users * @param int $serviceid service id @@ -104,25 +104,43 @@ class core_webservice_renderer extends plugin_renderer_base { */ public function admin_authorised_user_list($users, $serviceid) { global $CFG; - $html = $this->output->box_start('generalbox', 'alloweduserlist'); + + $listitems = []; + $extrafields = get_extra_user_fields(context_system::instance()); + foreach ($users as $user) { - $modifiedauthoriseduserurl = new moodle_url('/' . $CFG->admin . '/webservice/service_user_settings.php', - array('userid' => $user->id, 'serviceid' => $serviceid)); - $html .= html_writer::tag('a', $user->firstname . " " - . $user->lastname . ", " . s($user->email), - array('href' => $modifiedauthoriseduserurl)); - //add missing capabilities - if (!empty($user->missingcapabilities)) { - $html .= html_writer::tag('div', - get_string('usermissingcaps', 'webservice', $user->missingcapabilities) - . ' ' . $this->output->help_icon('missingcaps', 'webservice'), - array('class' => 'missingcaps', 'id' => 'usermissingcaps')); - $html .= html_writer::empty_tag('br'); - } else { - $html .= html_writer::empty_tag('br') . html_writer::empty_tag('br'); + $settingsurl = new moodle_url('/admin/webservice/service_user_settings.php', + ['userid' => $user->id, 'serviceid' => $serviceid]); + + $identity = []; + foreach ($extrafields as $extrafield) { + if (isset($user->{$extrafield})) { + $identity[] = s($user->{$extrafield}); + } } + $identity = $identity ? html_writer::div(implode(', ', $identity), 'small') : ''; + + $link = html_writer::link($settingsurl, fullname($user)); + + if (!empty($user->missingcapabilities)) { + $count = html_writer::span(count($user->missingcapabilities), 'badge badge-danger'); + $links = array_map(function($capname) { + return get_capability_docs_link((object)['name' => $capname]) . html_writer::div($capname, 'text-muted'); + }, $user->missingcapabilities); + $list = html_writer::alist($links); + $help = $this->output->help_icon('missingcaps', 'webservice'); + $missingcaps = print_collapsible_region(html_writer::div($list . $help, 'missingcaps'), 'small', + html_writer::random_id('usermissingcaps'), get_string('usermissingcaps', 'webservice', $count), '', true, true); + + } else { + $missingcaps = ''; + } + + $listitems[] = $link . $identity . $missingcaps; } - $html .= $this->output->box_end(); + + $html = html_writer::div(html_writer::alist($listitems), 'alloweduserlist'); + return $html; } From 2c177ae4c124bf3cb270618a511c127a34e37f9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Fri, 17 Apr 2020 16:14:53 +0200 Subject: [PATCH 10/15] MDL-67748 admin: Refactor tokens management as an external page No need to have a dedicated admin_setting_managewebservicetokens admin setting subclass, and yet do not use almost any of its features. So the patch merges the list and the forms handling into a single external page. Similarly, it feels like overkill to have a renderer method for a simple confirmation widget - especially given that the URLs were hard-coded in it. So that one was dropped. --- admin/settings/server.php | 13 +- admin/webservice/tokens.php | 194 +++++++++++++++-------------- lib/adminlib.php | 85 ------------- webservice/classes/token_table.php | 2 +- webservice/renderer.php | 23 ---- 5 files changed, 105 insertions(+), 212 deletions(-) diff --git a/admin/settings/server.php b/admin/settings/server.php index 1ae55f217d1..75e72ed3a08 100644 --- a/admin/settings/server.php +++ b/admin/settings/server.php @@ -607,15 +607,6 @@ if ($hassiteconfig) { } // Web services > Manage tokens. - $ADMIN->add('webservicesettings', new admin_externalpage('addwebservicetoken', new lang_string('managetokens', 'webservice'), - "{$CFG->wwwroot}/{$CFG->admin}/webservice/tokens.php", 'moodle/site:config', true)); - - $temp = new admin_settingpage('webservicetokens', new lang_string('managetokens', 'webservice')); - $temp->add(new admin_setting_managewebservicetokens()); - - if (empty($CFG->enablewebservices)) { - $temp->add(new admin_setting_heading('webservicesaredisabled', '', new lang_string('disabledwarning', 'webservice'))); - } - - $ADMIN->add('webservicesettings', $temp); + $ADMIN->add('webservicesettings', new admin_externalpage('webservicetokens', new lang_string('managetokens', 'webservice'), + new moodle_url('/admin/webservice/tokens.php'))); } diff --git a/admin/webservice/tokens.php b/admin/webservice/tokens.php index 21d74855f0c..fb95eb4e77b 100644 --- a/admin/webservice/tokens.php +++ b/admin/webservice/tokens.php @@ -1,6 +1,5 @@ . /** - * Web services tokens admin UI + * Web services / external tokens management UI. * - * @package webservice - * @author Jerome Mouneyrac - * @copyright 2009 Moodle Pty Ltd (http://moodle.com) - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @package core_webservice + * @category admin + * @copyright 2009 Jerome Mouneyrac + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -require_once('../../config.php'); + +require(__DIR__ . '/../../config.php'); require_once($CFG->libdir . '/adminlib.php'); require_once($CFG->libdir . '/externallib.php'); +require_once($CFG->dirroot . '/webservice/lib.php'); $action = optional_param('action', '', PARAM_ALPHANUMEXT); $tokenid = optional_param('tokenid', '', PARAM_SAFEDIR); $confirm = optional_param('confirm', 0, PARAM_BOOL); -admin_externalpage_setup('addwebservicetoken'); +admin_externalpage_setup('webservicetokens'); -//Deactivate the second 'Manage token' navigation node, and use the main 'Manage token' navigation node -$node = $PAGE->settingsnav->find('addwebservicetoken', navigation_node::TYPE_SETTING); -$newnode = $PAGE->settingsnav->find('webservicetokens', navigation_node::TYPE_SETTING); -if ($node && $newnode) { - $node->display = false; - $newnode->make_active(); +if ($action === 'create') { + $webservicemanager = new webservice(); + $mform = new \core_webservice\token_form(null, ['action' => 'create']); + $data = $mform->get_data(); + + if ($mform->is_cancelled()) { + redirect($PAGE->url); + + } else if ($data) { + ignore_user_abort(true); + + // Check the user is allowed for the service. + $selectedservice = $webservicemanager->get_external_service_by_id($data->service); + + if ($selectedservice->restrictedusers) { + $restricteduser = $webservicemanager->get_ws_authorised_user($data->service, $data->user); + + if (empty($restricteduser)) { + $allowuserurl = new moodle_url('/admin/webservice/service_users.php', ['id' => $selectedservice->id]); + $allowuserlink = html_writer::link($selectedservice->name, $allowuserurl); + $errormsg = $OUTPUT->notification(get_string('usernotallowed', 'webservice', $allowuserlink)); + } + } + + $user = \core_user::get_user($data->user, '*', MUST_EXIST); + \core_user::require_active_user($user); + + // Generate the token. + if (empty($errormsg)) { + external_generate_token(EXTERNAL_TOKEN_PERMANENT, $data->service, $data->user, context_system::instance(), + $data->validuntil, $data->iprestriction); + redirect($PAGE->url); + } + } + + echo $OUTPUT->header(); + echo $OUTPUT->heading(get_string('createtoken', 'webservice')); + if (!empty($errormsg)) { + echo $errormsg; + } + $mform->display(); + echo $OUTPUT->footer(); + die(); } +if ($action === 'delete') { + $webservicemanager = new webservice(); + $token = $webservicemanager->get_token_by_id_with_details($tokenid); -$tokenlisturl = new moodle_url("/" . $CFG->admin . "/settings.php", array('section' => 'webservicetokens')); + if ($token->creatorid != $USER->id) { + require_capability('moodle/webservice:managealltokens', context_system::instance()); + } -require_once($CFG->dirroot . "/webservice/lib.php"); -$webservicemanager = new webservice(); + if ($confirm && confirm_sesskey()) { + $webservicemanager->delete_user_ws_token($token->id); + redirect($PAGE->url); + } -switch ($action) { + echo $OUTPUT->header(); - case 'create': - $mform = new \core_webservice\token_form(null, array('action' => 'create')); - $data = $mform->get_data(); - if ($mform->is_cancelled()) { - redirect($tokenlisturl); - } else if ($data and confirm_sesskey()) { - ignore_user_abort(true); + echo $OUTPUT->confirm( + get_string('deletetokenconfirm', 'webservice', [ + 'user' => $token->firstname . ' ' . $token->lastname, + 'service' => $token->name, + ]), + new single_button(new moodle_url('/admin/webservice/tokens.php', [ + 'tokenid' => $token->id, + 'action' => 'delete', + 'confirm' => 1, + 'sesskey' => sesskey(), + ]), get_string('delete')), + $PAGE->url + ); - //check the the user is allowed for the service - $selectedservice = $webservicemanager->get_external_service_by_id($data->service); - if ($selectedservice->restrictedusers) { - $restricteduser = $webservicemanager->get_ws_authorised_user($data->service, $data->user); - if (empty($restricteduser)) { - $allowuserurl = new moodle_url('/' . $CFG->admin . '/webservice/service_users.php', - array('id' => $selectedservice->id)); - $allowuserlink = html_writer::tag('a', $selectedservice->name , array('href' => $allowuserurl)); - $errormsg = $OUTPUT->notification(get_string('usernotallowed', 'webservice', $allowuserlink)); - } - } - - //check if the user is deleted. unconfirmed, suspended or guest - $user = $DB->get_record('user', array('id' => $data->user)); - if ($user->id == $CFG->siteguest or $user->deleted or !$user->confirmed or $user->suspended) { - throw new moodle_exception('forbiddenwsuser', 'webservice'); - } - - //process the creation - if (empty($errormsg)) { - //TODO improvement: either move this function from externallib.php to webservice/lib.php - // either move most of webservicelib.php functions into externallib.php - // (create externalmanager class) MDL-23523 - external_generate_token(EXTERNAL_TOKEN_PERMANENT, $data->service, - $data->user, context_system::instance(), - $data->validuntil, $data->iprestriction); - redirect($tokenlisturl); - } - } - - //OUTPUT: create token form - echo $OUTPUT->header(); - echo $OUTPUT->heading(get_string('createtoken', 'webservice')); - if (!empty($errormsg)) { - echo $errormsg; - } - $mform->display(); - echo $OUTPUT->footer(); - die; - break; - - case 'delete': - $token = $webservicemanager->get_token_by_id_with_details($tokenid); - - if ($token->creatorid != $USER->id) { - require_capability("moodle/webservice:managealltokens", context_system::instance()); - } - - //Delete the token - if ($confirm and confirm_sesskey()) { - $webservicemanager->delete_user_ws_token($token->id); - redirect($tokenlisturl); - } - - ////OUTPUT: display delete token confirmation box - echo $OUTPUT->header(); - $renderer = $PAGE->get_renderer('core', 'webservice'); - echo $renderer->admin_delete_token_confirmation($token); - echo $OUTPUT->footer(); - die; - break; - - default: - //wrong url access - redirect($tokenlisturl); - break; + echo $OUTPUT->footer(); + die(); } + +echo $OUTPUT->header(); +echo $OUTPUT->heading(get_string('managetokens', 'core_webservice')); + +if (has_capability('moodle/webservice:managealltokens', context_system::instance())) { + echo html_writer::div(get_string('onlyseecreatedtokens', 'core_webservice'), 'alert alert-info'); +} + +$table = new \core_webservice\token_table('webservicetokens'); +$table->define_baseurl($PAGE->url); +$table->attributes['class'] = 'admintable generaltable'; +$table->data = []; +$table->out(30, false); + +echo $OUTPUT->footer(); + +// TODO Add button +//$tokenpageurl = "$CFG->wwwroot/$CFG->admin/webservice/tokens.php?sesskey=" . sesskey(); +// +//$return .= $OUTPUT->box_end(); +//// add a token to the table +//$return .= ""; +//$return .= get_string('add').""; diff --git a/lib/adminlib.php b/lib/adminlib.php index 923f9daab3a..b477d2e3eb8 100644 --- a/lib/adminlib.php +++ b/lib/adminlib.php @@ -10395,91 +10395,6 @@ class admin_setting_managewebserviceprotocols extends admin_setting { } } - -/** - * Special class for web service token administration. - * - * @author Jerome Mouneyrac - */ -class admin_setting_managewebservicetokens extends admin_setting { - - /** - * Calls parent::__construct with specific arguments - */ - public function __construct() { - $this->nosave = true; - parent::__construct('webservicestokenui', get_string('managetokens', 'webservice'), '', ''); - } - - /** - * Always returns true, does nothing - * - * @return true - */ - public function get_setting() { - return true; - } - - /** - * Always returns true, does nothing - * - * @return true - */ - public function get_defaultsetting() { - return true; - } - - /** - * Always returns '', does not write anything - * - * @return string Always returns '' - */ - public function write_setting($data) { - // do not write any setting - return ''; - } - - /** - * Builds the XHTML to display the control - * - * @param string $data Unused - * @param string $query - * @return string - */ - public function output_html($data, $query='') { - global $CFG, $OUTPUT; - - require_once($CFG->dirroot . '/webservice/classes/token_table.php'); - $baseurl = new moodle_url('/' . $CFG->admin . '/settings.php?section=webservicetokens'); - - $return = $OUTPUT->box_start('generalbox webservicestokenui'); - - if (has_capability('moodle/webservice:managealltokens', context_system::instance())) { - $return .= \html_writer::div(get_string('onlyseecreatedtokens', 'webservice')); - } - - $table = new \webservice\token_table('webservicetokens'); - $table->define_baseurl($baseurl); - $table->attributes['class'] = 'admintable generaltable'; // Any need changing? - $table->data = array(); - ob_start(); - $table->out(10, false); - $tablehtml = ob_get_contents(); - ob_end_clean(); - $return .= $tablehtml; - - $tokenpageurl = "$CFG->wwwroot/$CFG->admin/webservice/tokens.php?sesskey=" . sesskey(); - - $return .= $OUTPUT->box_end(); - // add a token to the table - $return .= ""; - $return .= get_string('add').""; - - return highlight($query, $return); - } -} - - /** * Colour picker * diff --git a/webservice/classes/token_table.php b/webservice/classes/token_table.php index 8e4e6bdf131..3f12802e4d9 100644 --- a/webservice/classes/token_table.php +++ b/webservice/classes/token_table.php @@ -22,7 +22,7 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -namespace webservice; +namespace core_webservice; defined('MOODLE_INTERNAL') || die; diff --git a/webservice/renderer.php b/webservice/renderer.php index 1f8a5e6a592..ebccbb9156e 100644 --- a/webservice/renderer.php +++ b/webservice/renderer.php @@ -184,29 +184,6 @@ class core_webservice_renderer extends plugin_renderer_base { $formcontinue, $formcancel); } - /** - * Display a confirmation page to delete a token - * - * @param stdClass $token Required properties: id (token id), firstname (user firstname), lastname (user lastname), name (service name) - * @return string html - */ - public function admin_delete_token_confirmation($token) { - global $CFG; - $optionsyes = array('tokenid' => $token->id, 'action' => 'delete', - 'confirm' => 1, 'sesskey' => sesskey()); - $optionsno = array('section' => 'webservicetokens', 'sesskey' => sesskey()); - $formcontinue = new single_button( - new moodle_url('/' . $CFG->admin . '/webservice/tokens.php', $optionsyes), - get_string('delete')); - $formcancel = new single_button( - new moodle_url('/' . $CFG->admin . '/settings.php', $optionsno), - get_string('cancel'), 'get'); - return $this->output->confirm(get_string('deletetokenconfirm', 'webservice', - (object) array('user' => $token->firstname . " " - . $token->lastname, 'service' => $token->name)), - $formcontinue, $formcancel); - } - /** * Display a list of functions for a given service * If the service is built-in, do not display remove/add operation (read-only) From bcd8692fef6caca731666b9e5f1692cecec91a27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Wed, 22 Apr 2020 17:55:27 +0200 Subject: [PATCH 11/15] MDL-67748 admin: Improve the display of tokens the user cannot see --- admin/webservice/tokens.php | 4 ---- lang/en/webservice.php | 2 +- webservice/classes/token_table.php | 2 +- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/admin/webservice/tokens.php b/admin/webservice/tokens.php index fb95eb4e77b..113dcee5c9e 100644 --- a/admin/webservice/tokens.php +++ b/admin/webservice/tokens.php @@ -115,10 +115,6 @@ if ($action === 'delete') { echo $OUTPUT->header(); echo $OUTPUT->heading(get_string('managetokens', 'core_webservice')); -if (has_capability('moodle/webservice:managealltokens', context_system::instance())) { - echo html_writer::div(get_string('onlyseecreatedtokens', 'core_webservice'), 'alert alert-info'); -} - $table = new \core_webservice\token_table('webservicetokens'); $table->define_baseurl($PAGE->url); $table->attributes['class'] = 'admintable generaltable'; diff --git a/lang/en/webservice.php b/lang/en/webservice.php index cdf7d0a87ab..bd996c9484c 100644 --- a/lang/en/webservice.php +++ b/lang/en/webservice.php @@ -130,7 +130,7 @@ $string['norequiredcapability'] = 'No required capability'; $string['notoken'] = 'The token list is empty.'; $string['onesystemcontrolling'] = 'Allow an external system to control Moodle'; $string['onesystemcontrollingdescription'] = 'The following steps help you to set up the Moodle web services to allow an external system to interact with Moodle. This includes setting up a token (security key) authentication method.'; -$string['onlyseecreatedtokens'] = 'Any token can be deleted, though you can only view tokens that you created.'; +$string['onlyseecreatedtokens'] = 'You can only view tokens that you created.'; $string['operation'] = 'Operation'; $string['optional'] = 'Optional'; $string['passwordisexpired'] = 'Password is expired.'; diff --git a/webservice/classes/token_table.php b/webservice/classes/token_table.php index 3f12802e4d9..b8730a73b19 100644 --- a/webservice/classes/token_table.php +++ b/webservice/classes/token_table.php @@ -165,7 +165,7 @@ class token_table extends \table_sql { global $USER; // Hide the token if it wasn't created by the current user. if ($data->creatorid != $USER->id) { - return '-'; + return \html_writer::tag('small', get_string('onlyseecreatedtokens', 'core_webservice'), ['class' => 'text-muted']); } return $data->token; From c4ad1bf2f62a0e500f55fc8ebf5c12a247a93ca5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Wed, 22 Apr 2020 22:22:13 +0200 Subject: [PATCH 12/15] MDL-67748 admin: Add filter to the WS tokens management page The patch adds ability to filter the list of token by the token value, the user and the service. Also the button to create a new token is made more prominent and easier to spot. --- admin/tests/behat/manage_tokens.feature | 2 +- admin/webservice/tokens.php | 57 +++++++++++--- lang/en/webservice.php | 3 + webservice/classes/token_filter.php | 100 ++++++++++++++++++++++++ webservice/classes/token_table.php | 74 ++++++++++++------ 5 files changed, 196 insertions(+), 40 deletions(-) create mode 100644 webservice/classes/token_filter.php diff --git a/admin/tests/behat/manage_tokens.feature b/admin/tests/behat/manage_tokens.feature index 20fda05f54d..0327ca4ba8e 100644 --- a/admin/tests/behat/manage_tokens.feature +++ b/admin/tests/behat/manage_tokens.feature @@ -16,7 +16,7 @@ Feature: Manage tokens @javascript Scenario: Add & delete a token Given I navigate to "Server > Web services > Manage tokens" in site administration - And I follow "Add" + And I press "Create token" And I set the field "User" to "Joe Bloggs" And I set the field "IP restriction" to "127.0.0.1" When I press "Save changes" diff --git a/admin/webservice/tokens.php b/admin/webservice/tokens.php index 113dcee5c9e..7e2599e2c60 100644 --- a/admin/webservice/tokens.php +++ b/admin/webservice/tokens.php @@ -31,6 +31,9 @@ require_once($CFG->dirroot . '/webservice/lib.php'); $action = optional_param('action', '', PARAM_ALPHANUMEXT); $tokenid = optional_param('tokenid', '', PARAM_SAFEDIR); $confirm = optional_param('confirm', 0, PARAM_BOOL); +$ftoken = optional_param('ftoken', '', PARAM_ALPHANUM); +$fusers = optional_param_array('fusers', [], PARAM_INT); +$fservices = optional_param_array('fservices', [], PARAM_INT); admin_externalpage_setup('webservicetokens'); @@ -52,9 +55,7 @@ if ($action === 'create') { $restricteduser = $webservicemanager->get_ws_authorised_user($data->service, $data->user); if (empty($restricteduser)) { - $allowuserurl = new moodle_url('/admin/webservice/service_users.php', ['id' => $selectedservice->id]); - $allowuserlink = html_writer::link($selectedservice->name, $allowuserurl); - $errormsg = $OUTPUT->notification(get_string('usernotallowed', 'webservice', $allowuserlink)); + $errormsg = $OUTPUT->notification(get_string('usernotallowed', 'webservice', $selectedservice->name)); } } @@ -112,21 +113,51 @@ if ($action === 'delete') { die(); } +// Pre-populate the form with the values that come as a part of the URL - typically when using the table_sql control +// links. +$filterdata = (object)[ + 'token' => $ftoken, + 'users' => $fusers, + 'services' => $fservices, +]; + +$filter = new \core_webservice\token_filter($PAGE->url, $filterdata); + +$filter->set_data($filterdata); + +if ($filter->is_submitted()) { + $filterdata = $filter->get_data(); + + if (isset($filterdata->resetbutton)) { + redirect($PAGE->url); + } +} + echo $OUTPUT->header(); echo $OUTPUT->heading(get_string('managetokens', 'core_webservice')); -$table = new \core_webservice\token_table('webservicetokens'); -$table->define_baseurl($PAGE->url); +echo html_writer::div($OUTPUT->render(new single_button(new moodle_url($PAGE->url, ['action' => 'create']), + get_string('createtoken', 'core_webservice'), 'get', true)), 'my-3'); + +$filter->display(); + +$table = new \core_webservice\token_table('webservicetokens', $filterdata); + +// In order to not lose the filter form values by clicking the table control links, make them part of the table's baseurl. +$baseurl = new moodle_url($PAGE->url, ['ftoken' => $filterdata->token]); + +foreach ($filterdata->users as $i => $userid) { + $baseurl->param("fusers[{$i}]", $userid); +} + +foreach ($filterdata->services as $i => $serviceid) { + $baseurl->param("fservices[{$i}]", $serviceid); +} + +$table->define_baseurl($baseurl); + $table->attributes['class'] = 'admintable generaltable'; $table->data = []; $table->out(30, false); echo $OUTPUT->footer(); - -// TODO Add button -//$tokenpageurl = "$CFG->wwwroot/$CFG->admin/webservice/tokens.php?sesskey=" . sesskey(); -// -//$return .= $OUTPUT->box_end(); -//// add a token to the table -//$return .= ""; -//$return .= get_string('add').""; diff --git a/lang/en/webservice.php b/lang/en/webservice.php index bd996c9484c..0ae2dd3d1ec 100644 --- a/lang/en/webservice.php +++ b/lang/en/webservice.php @@ -205,6 +205,9 @@ $string['token'] = 'Token'; $string['tokenauthlog'] = 'Token authentication'; $string['tokencreatedbyadmin'] = 'Can only be reset by administrator (*)'; $string['tokencreator'] = 'Creator'; +$string['tokenfilter'] = 'Tokens filter'; +$string['tokenfiltersubmit'] = 'Show only matching tokens'; +$string['tokenfilterreset'] = 'Show all tokens'; $string['unknownoptionkey'] = 'Unknown option key ({$a})'; $string['unnamedstringparam'] = 'A string parameter is unnamed.'; $string['updateusersettings'] = 'Update'; diff --git a/webservice/classes/token_filter.php b/webservice/classes/token_filter.php new file mode 100644 index 00000000000..8144fe62843 --- /dev/null +++ b/webservice/classes/token_filter.php @@ -0,0 +1,100 @@ +. + +/** + * Provides the {@see core_webservice\token_filter} class. + * + * @package core_webservice + * @copyright 2020 David Mudrák + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace core_webservice; + +use moodleform; + +/** + * Form allowing to filter displayed tokens. + * + * @copyright 2020 David Mudrák + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class token_filter extends moodleform { + + /** + * Defines the form fields. + */ + public function definition() { + global $DB; + + $mform = $this->_form; + $presetdata = $this->_customdata; + + $mform->addElement('header', 'tokenfilter', get_string('tokenfilter', 'webservice')); + + if (empty($presetdata->token) && empty($presetdata->users) && empty($presetdata->services)) { + $mform->setExpanded('tokenfilter', false); + } else { + $mform->setExpanded('tokenfilter', true); + } + + // Token. + $mform->addElement('text', 'token', get_string('token', 'core_webservice'), ['size' => 32]); + $mform->setType('token', PARAM_ALPHANUM); + + // User selector. + $attributes = [ + 'multiple' => true, + 'ajax' => 'core_user/form_user_selector', + 'valuehtmlcallback' => function($userid) { + global $DB, $OUTPUT; + + $context = \context_system::instance(); + $fields = \core\user_fields::for_name()->with_identity($context, false); + $record = \core_user::get_user($userid, 'id' . $fields->get_sql()->selects, MUST_EXIST); + + $user = (object)[ + 'id' => $record->id, + 'fullname' => fullname($record, has_capability('moodle/site:viewfullnames', $context)), + 'extrafields' => [], + ]; + + foreach ($fields->get_required_fields([\core\user_fields::PURPOSE_IDENTITY]) as $extrafield) { + $user->extrafields[] = (object)[ + 'name' => $extrafield, + 'value' => s($record->$extrafield) + ]; + } + + return $OUTPUT->render_from_template('core_user/form_user_selector_suggestion', $user); + }, + ]; + $mform->addElement('autocomplete', 'users', get_string('user'), [], $attributes); + + // Service selector. + $options = $DB->get_records_menu('external_services', null, '', 'id, name'); + $attributes = [ + 'multiple' => true, + ]; + $mform->addElement('autocomplete', 'services', get_string('service', 'webservice'), $options, $attributes); + + // Action buttons. + $mform->addGroup([ + $mform->createElement('submit', 'submitbutton', get_string('tokenfiltersubmit', 'core_webservice')), + $mform->createElement('submit', 'resetbutton', get_string('tokenfilterreset', 'core_webservice'), [], false), + ], 'actionbuttons', '', ' ', false); + } +} diff --git a/webservice/classes/token_table.php b/webservice/classes/token_table.php index b8730a73b19..b80e0dda587 100644 --- a/webservice/classes/token_table.php +++ b/webservice/classes/token_table.php @@ -47,11 +47,16 @@ class token_table extends \table_sql { /** @var bool $hasviewfullnames Does the user have the viewfullnames capability. */ protected $hasviewfullnames; + /** @var object */ + protected $filterdata; + /** * Sets up the table. + * * @param int $id The id of the table + * @param object $filterdata The data submitted by the {@see token_filter}. */ - public function __construct($id) { + public function __construct($id, $filterdata = null) { parent::__construct($id); // Get the context. @@ -61,6 +66,9 @@ class token_table extends \table_sql { $this->showalltokens = has_capability('moodle/webservice:managealltokens', $context); $this->hasviewfullnames = has_capability('moodle/site:viewfullnames', $context); + // Filter form values. + $this->filterdata = $filterdata; + // Define the headers and columns. $headers = []; $columns = []; @@ -227,41 +235,55 @@ class token_table extends \table_sql { $usernamefields = $userfieldsapi->get_sql('u', false, '', '', false)->selects; $creatorfields = $userfieldsapi->get_sql('c', false, 'creator', '', false)->selects; - $params = ["tokenmode" => EXTERNAL_TOKEN_PERMANENT]; + $params = ['tokenmode' => EXTERNAL_TOKEN_PERMANENT]; - // TODO: in order to let the administrator delete obsolete token, split the request in multiple request or use LEFT JOIN. + $selectfields = "SELECT t.id, t.token, t.iprestriction, t.validuntil, t.creatorid, + u.id AS userid, $usernamefields, + s.id AS serviceid, s.name, + $creatorfields "; - if ($this->showalltokens) { - // Show all tokens. - $sql = "SELECT t.id, t.token, u.id AS userid, $usernamefields, s.name, t.iprestriction, t.validuntil, s.id AS serviceid, - t.creatorid, $creatorfields - FROM {external_tokens} t, {user} u, {external_services} s, {user} c - WHERE t.tokentype = :tokenmode AND s.id = t.externalserviceid AND t.userid = u.id AND c.id = t.creatorid"; - $countsql = "SELECT COUNT(t.id) - FROM {external_tokens} t, {user} u, {external_services} s, {user} c - WHERE t.tokentype = :tokenmode AND s.id = t.externalserviceid AND t.userid = u.id AND c.id = t.creatorid"; - } else { + $selectcount = "SELECT COUNT(t.id) "; + + $sql = " FROM {external_tokens} t + JOIN {user} u ON u.id = t.userid + JOIN {external_services} s ON s.id = t.externalserviceid + JOIN {user} c ON c.id = t.creatorid + WHERE t.tokentype = :tokenmode"; + + if (!$this->showalltokens) { // Only show tokens created by the current user. - $sql = "SELECT t.id, t.token, u.id AS userid, $usernamefields, s.name, t.iprestriction, t.validuntil, s.id AS serviceid, - t.creatorid, $creatorfields - FROM {external_tokens} t, {user} u, {external_services} s, {user} c - WHERE t.creatorid=:userid AND t.tokentype = :tokenmode AND s.id = t.externalserviceid AND t.userid = u.id AND - c.id = t.creatorid"; - $countsql = "SELECT COUNT(t.id) - FROM {external_tokens} t, {user} u, {external_services} s, {user} c - WHERE t.creatorid=:userid AND t.tokentype = :tokenmode AND s.id = t.externalserviceid AND - t.userid = u.id AND c.id = t.creatorid"; - $params["userid"] = $USER->id; + $sql .= " AND t.creatorid = :userid"; + $params['userid'] = $USER->id; + } + + if ($this->filterdata->token !== '') { + $sql .= " AND " . $DB->sql_like("t.token", ":token"); + $params['token'] = "%" . $DB->sql_like_escape($this->filterdata->token) . "%"; + } + + if (!empty($this->filterdata->users)) { + list($sqlusers, $paramsusers) = $DB->get_in_or_equal($this->filterdata->users, SQL_PARAMS_NAMED, 'user'); + $sql .= " AND t.userid {$sqlusers}"; + $params += $paramsusers; + } + + if (!empty($this->filterdata->services)) { + list($sqlservices, $paramsservices) = $DB->get_in_or_equal($this->filterdata->services, SQL_PARAMS_NAMED, 'service'); + $sql .= " AND s.id {$sqlservices}"; + $params += $paramsservices; } $sort = $this->get_sql_sort(); + $sortsql = ''; + if ($sort) { - $sql = $sql . ' ORDER BY ' . $sort; + $sortsql = " ORDER BY {$sort}"; } - $total = $DB->count_records_sql($countsql, $params); + $total = $DB->count_records_sql($selectcount . $sql, $params); $this->pagesize($pagesize, $total); - $this->rawdata = $DB->get_recordset_sql($sql, $params, $this->get_page_start(), $this->get_page_size()); + $this->rawdata = $DB->get_recordset_sql($selectfields . $sql . $sortsql, $params, $this->get_page_start(), + $this->get_page_size()); } } From 6ea3588d82de965041500eb5d4e7a43dec258fa9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Fri, 24 Apr 2020 18:33:36 +0200 Subject: [PATCH 13/15] MDL-67748 admin: Show user identity fields on the manage tokens page On sites with many users, the fullname itself may not be unique identifier of users. Display the user identity fields below the token owner's name. --- webservice/classes/token_table.php | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/webservice/classes/token_table.php b/webservice/classes/token_table.php index b80e0dda587..2ffaebc8836 100644 --- a/webservice/classes/token_table.php +++ b/webservice/classes/token_table.php @@ -47,6 +47,9 @@ class token_table extends \table_sql { /** @var bool $hasviewfullnames Does the user have the viewfullnames capability. */ protected $hasviewfullnames; + /** @var array */ + protected $userextrafields; + /** @var object */ protected $filterdata; @@ -66,6 +69,9 @@ class token_table extends \table_sql { $this->showalltokens = has_capability('moodle/webservice:managealltokens', $context); $this->hasviewfullnames = has_capability('moodle/site:viewfullnames', $context); + // List of user identity fields. + $this->userextrafields = \core\user_fields::get_identity_fields(\context_system::instance(), false); + // Filter form values. $this->filterdata = $filterdata; @@ -142,9 +148,19 @@ class token_table extends \table_sql { public function col_fullname($data) { global $OUTPUT; + $identity = []; + + foreach ($this->userextrafields as $userextrafield) { + $identity[] = $data->$userextrafield; + } + $userprofilurl = new \moodle_url('/user/profile.php', ['id' => $data->userid]); $content = \html_writer::link($userprofilurl, fullname($data, $this->hasviewfullnames)); + if ($identity) { + $content .= \html_writer::div('' . implode(', ', $identity) . '', 'useridentity text-muted'); + } + // Make up list of capabilities that the user is missing for the given webservice. $webservicemanager = new \webservice(); $usermissingcaps = $webservicemanager->get_missing_capabilities_by_users([['id' => $data->userid]], $data->serviceid); @@ -156,7 +172,7 @@ class token_table extends \table_sql { }, $usermissingcaps[$data->userid]); $list = \html_writer::alist($links); $help = $OUTPUT->help_icon('missingcaps', 'webservice'); - $content .= print_collapsible_region(\html_writer::div($list . $help, 'missingcaps'), 'small', + $content .= print_collapsible_region(\html_writer::div($list . $help, 'missingcaps'), 'small mt-2', \html_writer::random_id('usermissingcaps'), get_string('usermissingcaps', 'webservice', $count), '', true, true); } @@ -235,6 +251,10 @@ class token_table extends \table_sql { $usernamefields = $userfieldsapi->get_sql('u', false, '', '', false)->selects; $creatorfields = $userfieldsapi->get_sql('c', false, 'creator', '', false)->selects; + if (!empty($this->userextrafields)) { + $usernamefields .= ',u.' . implode(',u.', $this->userextrafields); + } + $params = ['tokenmode' => EXTERNAL_TOKEN_PERMANENT]; $selectfields = "SELECT t.id, t.token, t.iprestriction, t.validuntil, t.creatorid, From b0fd376db3fde6ceec0a0ef3c0f3103d3660d6b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Fri, 24 Apr 2020 19:20:54 +0200 Subject: [PATCH 14/15] MDL-67748 admin: Do not show missing capabilities for mobile app service We used to display capabilities like "Manage any calendar entries", "Delete evidence", "Manage competency frameworks", "View hidden courses" and others as "Missing capabilities" for the mobile app service tokens. This gave dangerous impression that the app will not work for students without these capabilities granted. There are known cases of admins who started to grant all these caps to the Authenticated user role because they were afraid the app would not work for them. The problem here is that the official mobile app service includes some functions that have these capabilities declared as required. But they are not really required to use the app. Either the app makes its own clever checks of capabilities before calling the functions, or sometimes the capabilities are not even correctly declared. It is safer for everybody to display this information for custom services only where the risk of the falsely missing caps is lower and the information is more accurate. Also, the help text has been improved so it does not suggest that these capabilities must be always added. We do not know why the service has them declared. In some cases, a service has capabilities declared just because it makes use of them in the if-then fashion. Additionally, the patch also displays the service short name because it is actually needed to know. --- lang/en/webservice.php | 2 +- webservice/classes/token_table.php | 17 ++++++++++++++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/lang/en/webservice.php b/lang/en/webservice.php index 0ae2dd3d1ec..0017e1d85dc 100644 --- a/lang/en/webservice.php +++ b/lang/en/webservice.php @@ -118,7 +118,7 @@ $string['loginrequired'] = 'Restricted to logged-in users'; $string['manageprotocols'] = 'Manage protocols'; $string['managetokens'] = 'Manage tokens'; $string['missingcaps'] = 'Missing capabilities'; -$string['missingcaps_help'] = 'List of required capabilities for the service which the selected user does not have. Missing capabilities must be added to the user\'s role in order to use the service.'; +$string['missingcaps_help'] = 'List of capabilities declared by the service which the user does not have. Some service functionality may not be available without these capabilities.'; $string['missingpassword'] = 'Missing password'; $string['missingrequiredcapability'] = 'The capability {$a} is required.'; $string['missingusername'] = 'Missing username'; diff --git a/webservice/classes/token_table.php b/webservice/classes/token_table.php index 2ffaebc8836..deb8a7a816f 100644 --- a/webservice/classes/token_table.php +++ b/webservice/classes/token_table.php @@ -84,7 +84,7 @@ class token_table extends \table_sql { $headers[] = get_string('user'); $columns[] = 'fullname'; $headers[] = get_string('service', 'webservice'); - $columns[] = 'name'; + $columns[] = 'servicename'; $headers[] = get_string('iprestriction', 'webservice'); $columns[] = 'iprestriction'; $headers[] = get_string('validuntil', 'webservice'); @@ -165,7 +165,8 @@ class token_table extends \table_sql { $webservicemanager = new \webservice(); $usermissingcaps = $webservicemanager->get_missing_capabilities_by_users([['id' => $data->userid]], $data->serviceid); - if (!is_siteadmin($data->userid) && array_key_exists($data->userid, $usermissingcaps)) { + if ($data->serviceshortname <> MOODLE_OFFICIAL_MOBILE_SERVICE && !is_siteadmin($data->userid) + && array_key_exists($data->userid, $usermissingcaps)) { $count = \html_writer::span(count($usermissingcaps[$data->userid]), 'badge badge-danger'); $links = array_map(function($capname) { return get_capability_docs_link((object)['name' => $capname]) . \html_writer::div($capname, 'text-muted'); @@ -216,6 +217,16 @@ class token_table extends \table_sql { return \html_writer::link($creatorprofileurl, fullname((object)$user, $this->hasviewfullnames)); } + /** + * Format the service name column. + * + * @param \stdClass $data + * @return string + */ + public function col_servicename($data) { + return \html_writer::div(s($data->servicename)) . \html_writer::div(s($data->serviceshortname), 'small text-muted'); + } + /** * This function is used for the extra user fields. * @@ -259,7 +270,7 @@ class token_table extends \table_sql { $selectfields = "SELECT t.id, t.token, t.iprestriction, t.validuntil, t.creatorid, u.id AS userid, $usernamefields, - s.id AS serviceid, s.name, + s.id AS serviceid, s.name AS servicename, s.shortname AS serviceshortname, $creatorfields "; $selectcount = "SELECT COUNT(t.id) "; From 4a9f864969bf9e33dbac1d943bdda77609bbe7ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Mon, 15 Mar 2021 21:19:33 +0100 Subject: [PATCH 15/15] MDL-67748 admin: Add Behat tests for tokens filtering features Credit goes to Andrew for Behat data generators for webservices. Co-authored-by: Andrew Nicols --- admin/tests/behat/manage_tokens.feature | 79 ++++++++-- .../behat_core_webservice_generator.php | 57 +++++++ webservice/tests/generator/lib.php | 139 ++++++++++++++++++ 3 files changed, 260 insertions(+), 15 deletions(-) create mode 100644 webservice/tests/generator/behat_core_webservice_generator.php create mode 100644 webservice/tests/generator/lib.php diff --git a/admin/tests/behat/manage_tokens.feature b/admin/tests/behat/manage_tokens.feature index 0327ca4ba8e..e155dedeee0 100644 --- a/admin/tests/behat/manage_tokens.feature +++ b/admin/tests/behat/manage_tokens.feature @@ -1,27 +1,76 @@ @core @core_admin -Feature: Manage tokens - In order to manage webservice usage +Feature: Manage external services tokens + In order to manage external service usage As an admin - I need to be able to create and delete tokens + I need to be able to create, filter and delete tokens Background: Given the following "users" exist: - | username | password | firstname | lastname | - | testuser | testuser | Joe | Bloggs | - | testuser2 | testuser2 | TestFirstname | TestLastname | + | username | password | firstname | lastname | + | user1 | user1 | Firstname1 | Lastname1 | + | user2 | user2 | Firstname2 | Lastname2 | + | user3 | user3 | Firstname3 | Lastname3 | + | user4 | user4 | Firstname4 | Lastname4 | And I change window size to "small" - And I log in as "admin" - And I am on site homepage @javascript - Scenario: Add & delete a token - Given I navigate to "Server > Web services > Manage tokens" in site administration + Scenario: Add a token to user identified by name and then delete that token + Given I log in as "admin" + And I am on site homepage + And I navigate to "Server > Web services > Manage tokens" in site administration And I press "Create token" - And I set the field "User" to "Joe Bloggs" + And I set the field "User" to "Firstname1 Lastname1" + And I set the field "Service" to "Moodle mobile web service" And I set the field "IP restriction" to "127.0.0.1" When I press "Save changes" - Then I should see "Joe Bloggs" - And I should see "127.0.0.1" - And I follow "Delete" + Then I should see "Moodle mobile web service" in the "Firstname1 Lastname1" "table_row" + And I should see "127.0.0.1" in the "Firstname1 Lastname1" "table_row" + And I click on "Delete" "link" in the "Firstname1 Lastname1" "table_row" + And I should see "Do you really want to delete this web service token for Firstname1 Lastname1 on the service Moodle mobile web service?" And I press "Delete" - And I should not see "Joe Bloggs" + And "Firstname1 Lastname1" "table_row" should not exist + + @javascript + Scenario: Tokens can be filtered by user and by service + Given the following "core_webservice > Service" exists: + | name | Site information | + | shortname | siteinfo | + | enabled | 1 | + And the following "core_webservice > Service function" exists: + | service | siteinfo | + | functions | core_webservice_get_site_info | + And the following "core_webservice > Tokens" exist: + | user | service | + | user2 | siteinfo | + | user3 | moodle_mobile_app | + | user4 | siteinfo | + When I log in as "admin" + And I navigate to "Server > Web services > Manage tokens" in site administration + + # All created tokens are shown by default. + And "Firstname1 Lastname1" "table_row" should not exist + And I should see "Site information" in the "Firstname2 Lastname2" "table_row" + And I should see "Moodle mobile web service" in the "Firstname3 Lastname3" "table_row" + And I should see "Site information" in the "Firstname4 Lastname4" "table_row" + + # Filter tokens by user (note we can select the user by the identity field here). + When I click on "Tokens filter" "link" + And I set the field "User" to "user2@example.com" + And I press "Show only matching tokens" + Then "Firstname3 Lastname3" "table_row" should not exist + And "Firstname4 Lastname4" "table_row" should not exist + And I should see "Site information" in the "Firstname2 Lastname2" "table_row" + + # Reset the filter. + And I press "Show all tokens" + And I should see "Site information" in the "Firstname2 Lastname2" "table_row" + And I should see "Moodle mobile web service" in the "Firstname3 Lastname3" "table_row" + And I should see "Site information" in the "Firstname4 Lastname4" "table_row" + + # Filter tokens by service. + And I click on "Tokens filter" "link" + And I set the field "Service" to "Site information" + And I press "Show only matching tokens" + And I should see "Site information" in the "Firstname2 Lastname2" "table_row" + And I should see "Site information" in the "Firstname4 Lastname4" "table_row" + And "Firstname3 Lastname3" "table_row" should not exist diff --git a/webservice/tests/generator/behat_core_webservice_generator.php b/webservice/tests/generator/behat_core_webservice_generator.php new file mode 100644 index 00000000000..cf6f3744303 --- /dev/null +++ b/webservice/tests/generator/behat_core_webservice_generator.php @@ -0,0 +1,57 @@ +. + +/** + * Behat data generator for core_webservice. + * + * @package core_webservice + * @category test + * @copyright 2021 Andrew Nicols + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class behat_core_webservice_generator extends behat_generator_base { + + /** + * Get the list of creatable entities for a web service. + * + * @return array + */ + protected function get_creatable_entities(): array { + + return [ + 'Services' => [ + 'singular' => 'Service', + 'datagenerator' => 'service', + 'required' => ['name'], + ], + + 'Service functions' => [ + 'singular' => 'Service function', + 'datagenerator' => 'service_functions', + 'required' => ['service', 'functions'], + ], + + 'Tokens' => [ + 'singular' => 'Token', + 'datagenerator' => 'token', + 'required' => ['user'], + 'switchids' => [ + 'user' => 'userid', + ], + ], + ]; + } +} diff --git a/webservice/tests/generator/lib.php b/webservice/tests/generator/lib.php new file mode 100644 index 00000000000..9d877abc28b --- /dev/null +++ b/webservice/tests/generator/lib.php @@ -0,0 +1,139 @@ +. + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once(__DIR__ . '/../../lib.php'); + +/** + * Data generator for core_webservice plugin. + * + * @package core_webservice + * @category test + * @copyright 2021 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class core_webservice_generator extends component_generator_base { + /** + * Create a new webservice service. + * + * @param array $data + * @return stdClass + */ + public function create_service(array $data): \stdClass { + $webservicemanager = new webservice(); + + $requiredfields = [ + 'name', + 'shortname', + ]; + + foreach ($requiredfields as $fieldname) { + if (!array_key_exists($fieldname, $data)) { + throw new \coding_exception("Field '{$fieldname}' missing when creating new service"); + } + } + + $optionalfields = [ + 'requiredcapability' => '', + 'restrictedusers' => 0, + 'component' => '', + 'timemodified' => time(), + ]; + + foreach ($optionalfields as $fieldname => $value) { + if (!array_key_exists($fieldname, $data)) { + $data[$fieldname] = $value; + } + } + + $serviceid = $webservicemanager->add_external_service((object) $data); + + return $webservicemanager->get_external_service_by_id($serviceid); + } + + /** + * Associate a webservice function with service. + * + * @param array $data + */ + public function create_service_functions(array $data): void { + $webservicemanager = new webservice(); + + $requiredfields = [ + 'service', + 'functions', + ]; + + foreach ($requiredfields as $fieldname) { + if (!array_key_exists($fieldname, $data)) { + throw new \coding_exception("Field '{$fieldname}' missing when creating new service"); + } + } + + $service = $webservicemanager->get_external_service_by_shortname($data['service']); + + $functions = explode(',', $data['functions']); + foreach ($functions as $functionname) { + $functionname = trim($functionname); + $webservicemanager->add_external_function_to_service($functionname, $service->id); + } + } + + /** + * Create a new webservice token. + * + * @param array $data + */ + public function create_token(array $data): void { + $webservicemanager = new webservice(); + + $requiredfields = [ + 'userid', + 'service', + ]; + + foreach ($requiredfields as $fieldname) { + if (!array_key_exists($fieldname, $data)) { + throw new \coding_exception("Field '{$fieldname}' missing when creating new service"); + } + } + + $optionalfields = [ + 'context' => context_system::instance(), + 'validuntil' => 0, + 'iprestriction' => '', + ]; + + foreach ($optionalfields as $fieldname => $value) { + if (!array_key_exists($fieldname, $data)) { + $data[$fieldname] = $value; + } + } + + $service = $webservicemanager->get_external_service_by_shortname($data['service']); + + external_generate_token( + EXTERNAL_TOKEN_PERMANENT, + $service->id, + $data['userid'], + $data['context'], + $data['validuntil'], + $data['iprestriction'] + ); + } +}