Commit c07f15d7 authored by David Mudrák's avatar David Mudrák
Browse files

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.
parent 0bcaab32
...@@ -623,11 +623,16 @@ class webservice { ...@@ -623,11 +623,16 @@ class webservice {
* as the front end does not display it itself. In pratice, * as the front end does not display it itself. In pratice,
* admins would like the info, for more info you can follow: MDL-29962 * 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 * @param int $userid user id
* @return array * @return array
*/ */
public function get_user_capabilities($userid) { public function get_user_capabilities($userid) {
global $DB; global $DB;
debugging('webservice::get_user_capabilities() has been deprecated.', DEBUG_DEVELOPER);
//retrieve the user capabilities //retrieve the user capabilities
$sql = "SELECT DISTINCT rc.id, rc.capability FROM {role_capabilities} rc, {role_assignments} ra $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 = ?"; WHERE rc.roleid=ra.roleid AND ra.userid= ? AND rc.permission = ?";
...@@ -640,45 +645,97 @@ class webservice { ...@@ -640,45 +645,97 @@ class webservice {
} }
/** /**
* Get missing user capabilities for a given service * Get missing user capabilities for the given service's functions.
* 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
* *
* @param array $users users * Every external function can declare some required capabilities to allow for easier setup of the web services.
* @param int $serviceid service id * However, that is supposed to be used for informational admin report only. There is no automatic evaluation of
* @return array of missing capabilities, keys being the user ids * 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; global $DB;
$usersmissingcaps = array();
//retrieve capabilities required by the service // The following are default capabilities for all authenticated users and we will assume them granted.
$servicecaps = $this->get_service_required_capabilities($serviceid); $commoncaps = get_default_capabilities('user');
// 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;
}
}
}
if (empty($servicecaps)) {
return [];
}
//retrieve users missing capabilities // Prepare a list of user ids we want to check.
$userids = [];
foreach ($users as $user) { foreach ($users as $user) {
//cast user array into object to be a bit more flexible if (is_object($user) && isset($user->id)) {
if (is_array($user)) { $userids[$user->id] = true;
$user = (object) $user; } 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().');
} }
$usercaps = $this->get_user_capabilities($user->id); }
//detect the missing capabilities // Prepare a matrix of missing capabilities x users - consider them all missing by default.
foreach ($servicecaps as $functioname => $functioncaps) { foreach (array_keys($userids) as $userid) {
foreach ($functioncaps as $functioncap) { foreach (array_keys($servicecaps) as $capname) {
if (!array_key_exists($functioncap, $usercaps)) { $matrix[$userid][$capname] = true;
if (!isset($usersmissingcaps[$user->id]) }
or array_search($functioncap, $usersmissingcaps[$user->id]) === false) { }
$usersmissingcaps[$user->id][] = $functioncap;
} 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 $usersmissingcaps; return $matrix;
} }
/** /**
......
...@@ -193,6 +193,65 @@ class webservice_test extends advanced_testcase { ...@@ -193,6 +193,65 @@ class webservice_test extends advanced_testcase {
$this->assertEquals($before + 60, $token->lastaccess); $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. * Utility method that tests the parameter type of a method info's input/output parameter.
* *
......
...@@ -3,6 +3,12 @@ information provided here is intended especially for developers. ...@@ -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. 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 === === 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. * The class externallib_advanced_testcase, used in unit tests, has a new function called "configure_filters" to easily configure filters for external functions testing.
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment