Skip to content

GitLab

Commit 46f7b264 authored by sam marshall's avatar sam marshall
Browse files

MDL-59897 Accesslib: get_user_capability_course is slow

parent d8e9a23c
Hide whitespace changes
Inline Side-by-side
lib/accesslib.php
View file @ 46f7b264
......@@ -3784,7 +3784,9 @@ function count_role_users($roleid, context $context, $parent = false) {
/**
* This function gets the list of courses that this user has a particular capability in.
* It is still not very efficient.
*
* It is now reasonably efficient, but bear in mind that if there are users who have the capability
* everywhere, it may return an array of all courses.
*
* @param string $capability Capability in question
* @param int $userid User ID or null for current user
......@@ -3799,15 +3801,51 @@ function count_role_users($roleid, context $context, $parent = false) {
*/
function get_user_capability_course($capability, $userid = null, $doanything = true, $fieldsexceptid = '', $orderby = '',
$limit = 0) {
global $DB;
global $DB, $USER;
// Default to current user.
if (!$userid) {
$userid = $USER->id;
}
if ($doanything && is_siteadmin($userid)) {
// If the user is a site admin and $doanything is enabled then there is no need to restrict
// the list of courses.
$contextlimitsql = '';
$contextlimitparams = [];
} else {
// Gets SQL to limit contexts ('x' table) to those where the user has this capability.
list ($contextlimitsql, $contextlimitparams) = \core\access\get_user_capability_course_helper::get_sql(
$userid, $capability);
if (!$contextlimitsql) {
// If the does not have this capability in any context, return false without querying.
return false;
}
$contextlimitsql = 'WHERE' . $contextlimitsql;
unset($root);
}
// Convert fields list and ordering
$fieldlist = '';
if ($fieldsexceptid) {
$fields = array_map('trim', explode(',', $fieldsexceptid));
foreach($fields as $field) {
// Context fields have a different alias and are added further down.
if (strpos($field, 'ctx') !== 0) {
// Context fields have a different alias.
if (strpos($field, 'ctx') === 0) {
switch($field) {
case 'ctxlevel' :
$realfield = 'contextlevel';
break;
case 'ctxinstance' :
$realfield = 'instanceid';
break;
default:
$realfield = substr($field, 3);
break;
}
$fieldlist .= ',x.' . $realfield . ' AS ' . $field;
} else {
$fieldlist .= ',c.'.$field;
}
}
......@@ -3824,43 +3862,18 @@ function get_user_capability_course($capability, $userid = null, $doanything = t
$orderby = 'ORDER BY '.$orderby;
}
// Obtain a list of everything relevant about all courses including context.
// Note the result can be used directly as a context (we are going to), the course
// fields are just appended.
$contextpreload = context_helper::get_preload_record_columns_sql('x');
$contextlist = ['ctxid', 'ctxpath', 'ctxdepth', 'ctxlevel', 'ctxinstance'];
$unsetitems = $contextlist;
if ($fieldsexceptid) {
$coursefields = array_map('trim', explode(',', $fieldsexceptid));
$unsetitems = array_diff($contextlist, $coursefields);
}
$courses = array();
$rs = $DB->get_recordset_sql("SELECT c.id $fieldlist, $contextpreload
FROM {course} c
JOIN {context} x ON (c.id=x.instanceid AND x.contextlevel=".CONTEXT_COURSE.")
$orderby");
// Check capability for each course in turn
$rs = $DB->get_recordset_sql("
SELECT c.id $fieldlist
FROM {course} c
JOIN {context} x ON c.id = x.instanceid AND x.contextlevel = ?
$contextlimitsql
$orderby", array_merge([CONTEXT_COURSE], $contextlimitparams));
foreach ($rs as $course) {
// The preload_from_record() unsets the context related fields, but it is possible that our caller may
// want them returned too (so they can use them for their own context preloading). For that reason we
// pass a clone.
context_helper::preload_from_record(clone($course));
$context = context_course::instance($course->id);
if (has_capability($capability, $context, $userid, $doanything)) {
// Unset context fields if they were not asked for.
foreach ($unsetitems as $item) {
unset($course->$item);
}
// We've got the capability. Make the record look like a course record
// and store it
$courses[] = $course;
$limit--;
if ($limit == 0) {
break;
}
$courses[] = $course;
$limit--;
if ($limit == 0) {
break;
}
}
$rs->close();
......
lib/classes/access/get_user_capability_course_helper.php 0 → 100644
View file @ 46f7b264
<?php
// This file is part of Moodle - http://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 <http://www.gnu.org/licenses/>.
/**
* Helper functions to implement the complex get_user_capability_course function.
*
* @package core
* @copyright 2017 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
namespace core\access;
defined('MOODLE_INTERNAL') || die();
/**
* Helper functions to implement the complex get_user_capability_course function.
*
* @package core
* @copyright 2017 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class get_user_capability_course_helper {
/**
* Based on the given user's access data (roles) and system role definitions, works out
* an array of capability values at each relevant context for the given user and capability.
*
* This is organised by the effective context path (the one at which the capability takes
* effect) and then by role id.
*
* @param int $userid User id
* @param string $capability Capability e.g. 'moodle/course:view'
* @return array Array of capability constants, indexed by context path and role id
*/
protected static function get_capability_info_at_each_context($userid, $capability) {
// Get access data for user.
$accessdata = get_user_accessdata($userid);
// Get list of roles for user (any location) and information about these roles.
$roleids = [];
foreach ($accessdata['ra'] as $path => $roles) {
foreach ($roles as $roleid) {
$roleids[$roleid] = true;
}
}
$rdefs = get_role_definitions(array_keys($roleids));
// Get data for required capability at each context path where the user has a role that can
// affect it.
$systemcontext = \context_system::instance();
$pathroleperms = [];
foreach ($accessdata['ra'] as $userpath => $roles) {
foreach ($roles as $roleid) {
// Get role definition for that role.
foreach ($rdefs[$roleid] as $rolepath => $caps) {
// Ignore if this override/definition doesn't refer to the relevant cap.
if (!array_key_exists($capability, $caps)) {
continue;
}
// Check path is /1 or matches a path the user has.
if ($rolepath === '/' . $systemcontext->id) {
// Note /1 is listed first in the array so this entry will be overridden
// if there is an override for the role on this actual level.
$effectivepath = $userpath;
} else if (preg_match('~^' . $userpath . '($|/)~', $rolepath)) {
$effectivepath = $rolepath;
} else {
// Not inside an area where the user has the role, so ignore.
continue;
}
if (!array_key_exists($effectivepath, $pathroleperms)) {
$pathroleperms[$effectivepath] = [];
}
$pathroleperms[$effectivepath][$roleid] = $caps[$capability];
}
}
}
return $pathroleperms;
}
/**
* Calculates a permission tree based on an array of information about role permissions.
*
* The input parameter must be in the format returned by get_capability_info_at_each_context.
*
* The output is the root of a tree of stdClass objects with the fields 'path' (a context path),
* 'allow' (true or false), and 'children' (an array of similar objects).
*
* @param array $pathroleperms Array of permissions
* @return \stdClass Root object of permission tree
*/
protected static function calculate_permission_tree(array $pathroleperms) {
// Considering each discovered context path as an inflection point, evaluate the user's
// permission (based on all roles) at each point.
$pathallows = [];
$mindepth = 1000;
$maxdepth = 0;
foreach ($pathroleperms as $path => $roles) {
$evaluatedroleperms = [];
// Walk up the tree starting from this path.
$innerpath = $path;
while ($innerpath !== '') {
$roles = $pathroleperms[$innerpath];
// Evaluate roles at this path level.
foreach ($roles as $roleid => $perm) {
if (!array_key_exists($roleid, $evaluatedroleperms)) {
$evaluatedroleperms[$roleid] = $perm;
} else {
// The existing one is at a more specific level so it takes precedence
// UNLESS this is a prohibit.
if ($perm == CAP_PROHIBIT) {
$evaluatedroleperms[$roleid] = $perm;
}
}
}
// Go up to next path level (if any).
do {
$innerpath = substr($innerpath, 0, strrpos($innerpath, '/'));
if ($innerpath === '') {
// No higher level data.
break;
}
} while (!array_key_exists($innerpath, $pathroleperms));
}
// If we have an allow from any role, and no prohibits, then user can access this path,
// else not.
$allow = false;
foreach ($evaluatedroleperms as $perm) {
if ($perm == CAP_ALLOW) {
$allow = true;
} else if ($perm == CAP_PROHIBIT) {
$allow = false;
break;
}
}
// Store the result based on path and depth so that we can process in depth order in
// the next step.
$depth = strlen(preg_replace('~[^/]~', '', $path));
$mindepth = min($depth, $mindepth);
$maxdepth = max($depth, $maxdepth);
$pathallows[$depth][$path] = $allow;
}
// Organise into a tree structure, processing in depth order so that we have ancestors
// set up before we encounter their children.
$root = (object)['allow' => false, 'path' => null, 'children' => []];
$nodesbypath = [];
for ($depth = $mindepth; $depth <= $maxdepth; $depth++) {
// Skip any missing depth levels.
if (!array_key_exists($depth, $pathallows)) {
continue;
}
foreach ($pathallows[$depth] as $path => $allow) {
// Value for new tree node.
$leaf = (object)['allow' => $allow, 'path' => $path, 'children' => []];
// Try to find a place to join it on if there is one.
$ancestorpath = $path;
$found = false;
while ($ancestorpath) {
$ancestorpath = substr($ancestorpath, 0, strrpos($ancestorpath, '/'));
if (array_key_exists($ancestorpath, $nodesbypath)) {
$found = true;
break;
}
}
if ($found) {
$nodesbypath[$ancestorpath]->children[] = $leaf;
} else {
$root->children[] = $leaf;
}
$nodesbypath[$path] = $leaf;
}
}
return $root;
}
/**
* Given a permission tree (in calculate_permission_tree format), removes any subtrees that
* are negative from the root. For example, if a top-level node of the permission tree has
* 'false' permission then it is meaningless because the default permission is already false;
* this function will remove it. However, if there is a child within that node that is positive,
* then that will need to be kept.
*
* @param \stdClass $root Root object
* @return \stdClass Filtered tree root
*/
protected static function remove_negative_subtrees($root) {
// If a node 'starts' negative, we don't need it (as negative is the default) - extract only
// subtrees that start with a positive value.
$positiveroot = (object)['allow' => false, 'path' => null, 'children' => []];
$consider = [$root];
while ($consider) {
$first = array_shift($consider);
foreach ($first->children as $node) {
if ($node->allow) {
// Add directly to new root.
$positiveroot->children[] = $node;
} else {
// Consider its children for adding to root (if there are any positive ones).
$consider[] = $node;
}
}
}
return $positiveroot;
}
/**
* Removes duplicate nodes of a tree - where a child node has the same permission as its
* parent.
*
* @param \stdClass $parent Tree root node
*/
protected static function remove_duplicate_nodes($parent) {
$length = count($parent->children);
$index = 0;
while ($index < $length) {
$child = $parent->children[$index];
if ($child->allow === $parent->allow) {
// Remove child node, but add its children to this node instead.
array_splice($parent->children, $index, 1);
$length--;
$index--;
foreach ($child->children as $grandchild) {
$parent->children[] = $grandchild;
$length++;
}
} else {
// Keep child node, but recurse to remove its unnecessary children.
self::remove_duplicate_nodes($child);
}
$index++;
}
}
/**
* Gets a permission tree for the given user and capability, representing the value of that
* capability at different contexts across the system. The tree will be simplified as far as
* possible.
*
* The output is the root of a tree of stdClass objects with the fields 'path' (a context path),
* 'allow' (true or false), and 'children' (an array of similar objects).
*
* @param int $userid User id
* @param string $capability Capability e.g. 'moodle/course:view'
* @return \stdClass Root node of tree
*/
protected static function get_tree($userid, $capability) {
// Extract raw capability data for this user and capability.
$pathroleperms = self::get_capability_info_at_each_context($userid, $capability);
// Convert the raw data into a permission tree based on context.
$root = self::calculate_permission_tree($pathroleperms);
unset($pathroleperms);
// Simplify the permission tree by removing unnecessary nodes.
$root = self::remove_negative_subtrees($root);
self::remove_duplicate_nodes($root);
// Return the tree.
return $root;
}
/**
* Creates SQL suitable for restricting by contexts listed in the given permission tree.
*
* This function relies on the permission tree being in the format created by get_tree.
* Specifically, all the children of the root element must be set to 'allow' permission,
* children of those children must be 'not allow', children of those grandchildren 'allow', etc.
*
* @param \stdClass $parent Root node of permission tree
* @return array Two-element array of SQL (containing ? placeholders) and then a params array
*/
protected static function create_sql($parent) {
global $DB;
$sql = '';
$params = [];
if ($parent->path !== null) {
// Except for the root element, create the condition that it applies to the context of
// this element (or anything within it).
$sql = ' (x.path = ? OR ' . $DB->sql_like('x.path', '?') .')';
$params[] = $parent->path;
$params[] = $parent->path . '/%';
if ($parent->children) {
// When there are children, these are assumed to have the opposite sign i.e. if we
// are allowing the parent, we are not allowing the children, and vice versa. So
// the 'OR' clause for children will be inside this 'AND NOT'.
$sql .= ' AND NOT (';
}
} else if (count($parent->children) > 1) {
// Place brackets in the query when it is going to be an OR of multiple conditions.
$sql .= ' (';
}
if ($parent->children) {
$first = true;
foreach ($parent->children as $child) {
if ($first) {
$first = false;
} else {
$sql .= ' OR';
}
// Recuse to get the child requirements - this will be the check that the context
// is within the child, plus possibly and 'AND NOT' for any different contexts
// within the child.
list ($childsql, $childparams) = self::create_sql($child);
$sql .= $childsql;
$params = array_merge($params, $childparams);
}
// Close brackets if opened above.
if ($parent->path !== null || count($parent->children) > 1) {
$sql .= ')';
}
}
return [$sql, $params];
}
/**
* Gets SQL to restrict a query to contexts in which the user has a capability.
*
* This returns an array with two elements (SQL containing ? placeholders, and a params array).
* The SQL is intended to be used as part of a WHERE clause. It relies on the prefix 'x' being
* used for the Moodle context table.
*
* If the user does not have the permission anywhere at all (so that there is no point doing
* the query) then the two returned values will both be false.
*
* @param int $userid User id
* @param string $capability Capability e.g. 'moodle/course:view'
* @return array Two-element array of SQL (containing ? placeholders) and then a params array
*/
public static function get_sql($userid, $capability) {
// Get a tree of capability permission at various contexts for current user.
$root = self::get_tree($userid, $capability);
// The root node always has permission false. If there are no child nodes then the user
// cannot access anything.
if (!$root->children) {
return [false, false];
}
// Get SQL to limit contexts based on the permission tree.
return self::create_sql($root);
}
}
lib/tests/accesslib_test.php
View file @ 46f7b264
......@@ -1696,6 +1696,165 @@ class core_accesslib_testcase extends advanced_testcase {
$this->assertFalse(has_all_capabilities($sca, $coursecontext, 0));
}
/**
* Tests get_user_capability_course() which checks a capability across all courses.
*/
public function test_get_user_capability_course() {
global $CFG, $USER;
$this->resetAfterTest();
$generator = $this->getDataGenerator();
$cap = 'moodle/course:view';
// Create a role which allows course:view and one that prohibits it, and one neither.
$allowroleid = $generator->create_role();
$prohibitroleid = $generator->create_role();
$emptyroleid = $generator->create_role();
$systemcontext = context_system::instance();
assign_capability($cap, CAP_ALLOW, $allowroleid, $systemcontext->id);
assign_capability($cap, CAP_PROHIBIT, $prohibitroleid, $systemcontext->id);
// Create two categories (nested).
$cat1 = $generator->create_category();
$cat2 = $generator->create_category(['parent' => $cat1->id]);
// Create six courses - two in cat1, two in cat2, and two in default category.
$c1 = $generator->create_course(['category' => $cat1->id, 'shortname' => 'Z']);
$c2 = $generator->create_course(['category' => $cat1->id, 'shortname' => 'Y']);
$c3 = $generator->create_course(['category' => $cat2->id, 'shortname' => 'X']);
$c4 = $generator->create_course(['category' => $cat2->id]);
$c5 = $generator->create_course();
$c6 = $generator->create_course();
// Category overrides: in cat 1, empty role is allowed; in cat 2, empty role is prevented.
assign_capability($cap, CAP_ALLOW, $emptyroleid,
context_coursecat::instance($cat1->id)->id);
assign_capability($cap, CAP_PREVENT, $emptyroleid,
context_coursecat::instance($cat2->id)->id);
// Course overrides: in C5, allow role is prevented; in C6, empty role is prohibited; in
// C3, empty role is allowed.
assign_capability($cap, CAP_PREVENT, $allowroleid,
context_course::instance($c5->id)->id);
assign_capability($cap, CAP_PROHIBIT, $emptyroleid,
context_course::instance($c6->id)->id);
assign_capability($cap, CAP_ALLOW, $emptyroleid,
context_course::instance($c3->id)->id);
// User 1 has no roles except default user role.
$u1 = $generator->create_user();
// It returns false (annoyingly) if there are no courses.
$this->assertFalse(get_user_capability_course($cap, $u1->id, true, '', 'id'));
// Final override: in C1, default user role is allowed.
assign_capability($cap, CAP_ALLOW, $CFG->defaultuserroleid,
context_course::instance($c1->id)->id);
// Should now get C1 only.
$courses = get_user_capability_course($cap, $u1->id, true, '', 'id');
$this->assert_course_ids([$c1->id], $courses);
// User 2 has allow role (system wide).
$u2 = $generator->create_user();
role_assign($allowroleid, $u2->id, $systemcontext->id);
// Should get everything except C5.
$courses = get_user_capability_course($cap, $u2->id, true, '', 'id');
$this->assert_course_ids([SITEID, $c1->id, $c2->id, $c3->id, $c4->id, $c6->id], $courses);
// User 3 has empty role (system wide).
$u3 = $generator->create_user();
role_assign($emptyroleid, $u3->id, $systemcontext->id);
// Should get cat 1 courses but not cat2, except C3.
$courses = get_user_capability_course($cap, $u3->id, true, '', 'id');
$this->assert_course_ids([$c1->id, $c2->id, $c3->id], $courses);
// User 4 has allow and empty role (system wide).
$u4 = $generator->create_user();
role_assign($allowroleid, $u4->id, $systemcontext->id);
role_assign($emptyroleid, $u4->id, $systemcontext->id);
// Should get everything except C5 and C6.
$courses = get_user_capability_course($cap, $u4->id, true, '', 'id');
$this->assert_course_ids([SITEID, $c1->id, $c2->id, $c3->id, $c4->id], $courses);
// User 5 has allow role in default category only.
$u5 = $generator->create_user();
role_assign($allowroleid, $u5->id, context_coursecat::instance($c5->category)->id);
// Should get C1 and the default category courses but not C5.
$courses = get_user_capability_course($cap, $u5->id, true, '', 'id');
$this->assert_course_ids([$c1->id, $c6->id], $courses);
// User 6 has a bunch of course roles: prohibit role in C1, empty role in C3, allow role in
// C6.
$u6 = $generator->create_user();
role_assign($prohibitroleid, $u6->id, context_course::instance($c1->id)->id);
role_assign($emptyroleid, $u6->id, context_course::instance($c3->id)->id);
role_assign($allowroleid, $u6->id, context_course::instance($c5->id)->id);
// Should get C3 only because the allow role is prevented in C5.
$courses = get_user_capability_course($cap, $u6->id, true, '', 'id');
$this->assert_course_ids([$c3->id], $courses);
// Admin user gets everything....
$courses = get_user_capability_course($cap, get_admin()->id, true, '', 'id');
$this->assert_course_ids([SITEID, $c1->id, $c2->id, $c3->id, $c4->id, $c5->id, $c6->id],
$courses);