mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-04-13 04:22:07 +02:00
Code Issues Projects Releases Wiki Activity

MDL-68402 accesslib: fix get_with_capability_join logic

Browse Source 
In fact, rather than fix the old logic, I noticed that the correct
logic was already implemented in get_users_by_capability. So, I
refactored to extract the working version into a function, which it
turns out can have exactly the same API as get_with_capability_join,
which was convenient.
This commit is contained in:
Tim Hunt 2020-04-14 17:06:13 +01:00
parent cd391f9922
commit 0d3bdb94bc
3 changed files with 226 additions and 267 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

397
lib/accesslib.php
View File

@ -3427,36 +3427,25 @@ function set_role_contextlevels($roleid, array $contextlevels) {
}
/**
* Who has this capability in this context?
* Gets sql joins for finding users with capability in the given context.
*
* This can be a very expensive call - use sparingly and keep
* the results if you are going to need them again soon.
*
* Note if $fields is empty this function attempts to get u.*
* which can get rather large - and has a serious perf impact
* on some DBs.
*
* @param context $context
* @param string|array $capability - capability name(s)
* @param string $fields - fields to be pulled. The user table is aliased to 'u'. u.id MUST be included.
* @param string $sort - the sort order. Default is lastaccess time.
* @param mixed $limitfrom - number of records to skip (offset)
* @param mixed $limitnum - number of records to fetch
* @param string|array $groups - single group or array of groups - only return
* users who are in one of these group(s).
* @param string|array $exceptions - list of users to exclude, comma separated or array
* @param bool $doanything_ignored not used any more, admin accounts are never returned
* @param bool $view_ignored - use get_enrolled_sql() instead
* @param bool $useviewallgroups if $groups is set the return users who
* have capability both $capability and moodle/site:accessallgroups
* in this context, as well as users who have $capability and who are
* in $groups.
* @return array of user records
* @param context $context Context for the join.
* @param string|array $capability Capability name or array of names.
* If an array is provided then this is the equivalent of a logical 'OR',
* i.e. the user needs to have one of these capabilities.
* @param string $useridcolumn e.g. 'u.id'.
* @return \core\dml\sql_join Contains joins, wheres, params.
* This function will set ->cannotmatchanyrows if applicable.
* This may let you skip doing a DB query.
*/
function get_users_by_capability(context $context, $capability, $fields = '', $sort = '', $limitfrom = '', $limitnum = '',
$groups = '', $exceptions = '', $doanything_ignored = null, $view_ignored = null, $useviewallgroups = false) {
function get_with_capability_join(context $context, $capability, $useridcolumn) {
global $CFG, $DB;
// Add a unique prefix to param names to ensure they are unique.
static $i = 0;
$i++;
$paramprefix = 'eu' . $i . '_';
$defaultuserroleid = isset($CFG->defaultuserroleid) ? $CFG->defaultuserroleid : 0;
$defaultfrontpageroleid = isset($CFG->defaultfrontpageroleid) ? $CFG->defaultfrontpageroleid : 0;
@ -3464,16 +3453,8 @@ function get_users_by_capability(context $context, $capability, $fields = '', $s
$ctxids = str_replace('/', ',', $ctxids);
// Context is the frontpage
$iscoursepage = false; // coursepage other than fp
$isfrontpage = false;
if ($context->contextlevel == CONTEXT_COURSE) {
if ($context->instanceid == SITEID) {
$isfrontpage = true;
} else {
$iscoursepage = true;
}
}
$isfrontpage = ($isfrontpage || is_inside_frontpage($context));
$isfrontpage = $context->contextlevel == CONTEXT_COURSE && $context->instanceid == SITEID;
$isfrontpage = $isfrontpage || is_inside_frontpage($context);
$caps = (array)$capability;
@ -3482,8 +3463,8 @@ function get_users_by_capability(context $context, $capability, $fields = '', $s
// we need to find out all roles that have these capabilities either in definition or in overrides
$defs = array();
list($incontexts, $params) = $DB->get_in_or_equal($contextids, SQL_PARAMS_NAMED, 'con');
list($incaps, $params2) = $DB->get_in_or_equal($caps, SQL_PARAMS_NAMED, 'cap');
list($incontexts, $params) = $DB->get_in_or_equal($contextids, SQL_PARAMS_NAMED, $paramprefix . 'con');
list($incaps, $params2) = $DB->get_in_or_equal($caps, SQL_PARAMS_NAMED, $paramprefix . 'cap');
// Check whether context locking is enabled.
// Filter out any write capability if this is the case.
@ -3558,7 +3539,7 @@ function get_users_by_capability(context $context, $capability, $fields = '', $s
if (empty($needed)) {
// there can not be anybody if no roles match this request
return array();
return new \core\dml\sql_join('', '1 = 2', [], true);
}
if (empty($prohibited)) {
@ -3573,76 +3554,15 @@ function get_users_by_capability(context $context, $capability, $fields = '', $s
unset($n);
}
// ***** Set up default fields ******
if (empty($fields)) {
if ($iscoursepage) {
$fields = 'u.*, ul.timeaccess AS lastaccess';
} else {
$fields = 'u.*';
}
} else {
if ($CFG->debugdeveloper && strpos($fields, 'u.*') === false && strpos($fields, 'u.id') === false) {
debugging('u.id must be included in the list of fields passed to get_users_by_capability().', DEBUG_DEVELOPER);
}
}
// Set up default sort
if (empty($sort)) { // default to course lastaccess or just lastaccess
if ($iscoursepage) {
$sort = 'ul.timeaccess';
} else {
$sort = 'u.lastaccess';
}
}
// Prepare query clauses
$wherecond = array();
$params = array();
$joins = array();
// User lastaccess JOIN
if ((strpos($sort, 'ul.timeaccess') === false) and (strpos($fields, 'ul.timeaccess') === false)) {
// user_lastaccess is not required MDL-13810
} else {
if ($iscoursepage) {
$joins[] = "LEFT OUTER JOIN {user_lastaccess} ul ON (ul.userid = u.id AND ul.courseid = {$context->instanceid})";
} else {
throw new coding_exception('Invalid sort in get_users_by_capability(), ul.timeaccess allowed only for course contexts.');
}
}
// We never return deleted users or guest account.
$wherecond[] = "u.deleted = 0 AND u.id <> :guestid";
$params['guestid'] = $CFG->siteguest;
// Groups
if ($groups) {
$groups = (array)$groups;
list($grouptest, $grpparams) = $DB->get_in_or_equal($groups, SQL_PARAMS_NAMED, 'grp');
$joins[] = "LEFT OUTER JOIN (SELECT DISTINCT userid
FROM {groups_members}
WHERE groupid $grouptest
) gm ON gm.userid = u.id";
$params = array_merge($params, $grpparams);
$grouptest = 'gm.userid IS NOT NULL';
if ($useviewallgroups) {
$viewallgroupsusers = get_users_by_capability($context, 'moodle/site:accessallgroups', 'u.id, u.id', '', '', '', '', $exceptions);
if (!empty($viewallgroupsusers)) {
$grouptest .= ' OR u.id IN (' . implode(',', array_keys($viewallgroupsusers)) . ')';
}
}
$wherecond[] = "($grouptest)";
}
// User exceptions
if (!empty($exceptions)) {
$exceptions = (array)$exceptions;
list($exsql, $exparams) = $DB->get_in_or_equal($exceptions, SQL_PARAMS_NAMED, 'exc', false);
$params = array_merge($params, $exparams);
$wherecond[] = "u.id $exsql";
}
$deletedusercolumn = substr($useridcolumn, 0, -2) . 'deleted'; // Hack to get the deleted user column without an API change.
$wherecond[] = "$deletedusercolumn = 0 AND $useridcolumn <> :{$paramprefix}guestid";
$params[$paramprefix . 'guestid'] = $CFG->siteguest;
// now add the needed and prohibited roles conditions as joins
if (!empty($needed['any'])) {
@ -3654,7 +3574,7 @@ function get_users_by_capability(context $context, $capability, $fields = '', $s
FROM {role_assignments}
WHERE contextid IN ($ctxids)
AND roleid IN (".implode(',', array_keys($needed['any'])) .")
) ra ON ra.userid = u.id";
) ra ON ra.userid = $useridcolumn";
}
} else {
$unions = array();
@ -3699,7 +3619,7 @@ function get_users_by_capability(context $context, $capability, $fields = '', $s
}
if (!$everybody) {
if ($unions) {
$joins[] = "JOIN (SELECT DISTINCT userid FROM ( ".implode(' UNION ', $unions)." ) us) ra ON ra.userid = u.id";
$joins[] = "JOIN (SELECT DISTINCT userid FROM ( ".implode(' UNION ', $unions)." ) us) ra ON ra.userid = $useridcolumn";
} else {
// only prohibits found - nobody can be matched
$wherecond[] = "1 = 2";
@ -3707,6 +3627,116 @@ function get_users_by_capability(context $context, $capability, $fields = '', $s
}
}
return new \core\dml\sql_join(implode("\n", $joins), implode(" AND ", $wherecond), $params);
}
/**
* Who has this capability in this context?
*
* This can be a very expensive call - use sparingly and keep
* the results if you are going to need them again soon.
*
* Note if $fields is empty this function attempts to get u.*
* which can get rather large - and has a serious perf impact
* on some DBs.
*
* @param context $context
* @param string|array $capability - capability name(s)
* @param string $fields - fields to be pulled. The user table is aliased to 'u'. u.id MUST be included.
* @param string $sort - the sort order. Default is lastaccess time.
* @param mixed $limitfrom - number of records to skip (offset)
* @param mixed $limitnum - number of records to fetch
* @param string|array $groups - single group or array of groups - only return
* users who are in one of these group(s).
* @param string|array $exceptions - list of users to exclude, comma separated or array
* @param bool $doanything_ignored not used any more, admin accounts are never returned
* @param bool $view_ignored - use get_enrolled_sql() instead
* @param bool $useviewallgroups if $groups is set the return users who
* have capability both $capability and moodle/site:accessallgroups
* in this context, as well as users who have $capability and who are
* in $groups.
* @return array of user records
*/
function get_users_by_capability(context $context, $capability, $fields = '', $sort = '', $limitfrom = '', $limitnum = '',
$groups = '', $exceptions = '', $doanything_ignored = null, $view_ignored = null, $useviewallgroups = false) {
global $CFG, $DB;
// Context is a course page other than the frontpage
$iscoursepage = $context->contextlevel == CONTEXT_COURSE && $context->instanceid != SITEID;
// Get the bits of SQL relating to capabilities.
$sqljoin = get_with_capability_join($context, $capability, 'u.id');
if ($sqljoin->cannotmatchanyrows) {
return [];
}
// ***** Set up default fields ******
if (empty($fields)) {
if ($iscoursepage) {
$fields = 'u.*, ul.timeaccess AS lastaccess';
} else {
$fields = 'u.*';
}
} else {
if ($CFG->debugdeveloper && strpos($fields, 'u.*') === false && strpos($fields, 'u.id') === false) {
debugging('u.id must be included in the list of fields passed to get_users_by_capability().', DEBUG_DEVELOPER);
}
}
// Set up default sort
if (empty($sort)) { // default to course lastaccess or just lastaccess
if ($iscoursepage) {
$sort = 'ul.timeaccess';
} else {
$sort = 'u.lastaccess';
}
}
// Prepare query clauses
$wherecond = [$sqljoin->wheres];
$params = $sqljoin->params;
$joins = [$sqljoin->joins];
// User lastaccess JOIN
if ((strpos($sort, 'ul.timeaccess') === false) and (strpos($fields, 'ul.timeaccess') === false)) {
// user_lastaccess is not required MDL-13810
} else {
if ($iscoursepage) {
$joins[] = "LEFT OUTER JOIN {user_lastaccess} ul ON (ul.userid = u.id AND ul.courseid = {$context->instanceid})";
} else {
throw new coding_exception('Invalid sort in get_users_by_capability(), ul.timeaccess allowed only for course contexts.');
}
}
// Groups
if ($groups) {
$groups = (array)$groups;
list($grouptest, $grpparams) = $DB->get_in_or_equal($groups, SQL_PARAMS_NAMED, 'grp');
$joins[] = "LEFT OUTER JOIN (SELECT DISTINCT userid
FROM {groups_members}
WHERE groupid $grouptest
) gm ON gm.userid = u.id";
$params = array_merge($params, $grpparams);
$grouptest = 'gm.userid IS NOT NULL';
if ($useviewallgroups) {
$viewallgroupsusers = get_users_by_capability($context, 'moodle/site:accessallgroups', 'u.id, u.id', '', '', '', '', $exceptions);
if (!empty($viewallgroupsusers)) {
$grouptest .= ' OR u.id IN (' . implode(',', array_keys($viewallgroupsusers)) . ')';
}
}
$wherecond[] = "($grouptest)";
}
// User exceptions
if (!empty($exceptions)) {
$exceptions = (array)$exceptions;
list($exsql, $exparams) = $DB->get_in_or_equal($exceptions, SQL_PARAMS_NAMED, 'exc', false);
$params = array_merge($params, $exparams);
$wherecond[] = "u.id $exsql";
}
// Collect WHERE conditions and needed joins
$where = implode(' AND ', $wherecond);
if ($where !== '') {
@ -7586,160 +7616,3 @@ function get_with_capability_sql(context $context, $capability) {
return array($sql, $capjoin->params);
}
/**
* Gets sql joins for finding users with capability in the given context
*
* @param context $context Context for the join
* @param string|array $capability Capability name or array of names.
* If an array is provided then this is the equivalent of a logical 'OR',
* i.e. the user needs to have one of these capabilities.
* @param string $useridcolumn e.g. 'u.id'
* @return \core\dml\sql_join Contains joins, wheres, params
*/
function get_with_capability_join(context $context, $capability, $useridcolumn) {
global $DB, $CFG;
// Use unique prefix just in case somebody makes some SQL magic with the result.
static $i = 0;
$i++;
$prefix = 'eu' . $i . '_';
// First find the course context.
$coursecontext = $context->get_course_context();
$isfrontpage = ($coursecontext->instanceid == SITEID);
$joins = array();
$wheres = array();
$params = array();
list($contextids, $contextpaths) = get_context_info_list($context);
list($incontexts, $cparams) = $DB->get_in_or_equal($contextids, SQL_PARAMS_NAMED, 'ctx');
list($incaps, $capsparams) = $DB->get_in_or_equal($capability, SQL_PARAMS_NAMED, 'cap');
// Check whether context locking is enabled.
// Filter out any write capability if this is the case.
$excludelockedcaps = '';
$excludelockedcapsparams = [];
if (!empty($CFG->contextlocking) && $context->locked) {
$excludelockedcaps = 'AND (cap.captype = :capread OR cap.name = :managelockscap)';
$excludelockedcapsparams['capread'] = 'read';
$excludelockedcapsparams['managelockscap'] = 'moodle/site:managecontextlocks';
}
$defs = array();
$sql = "SELECT rc.id, rc.roleid, rc.permission, ctx.path
FROM {role_capabilities} rc
JOIN {capabilities} cap ON rc.capability = cap.name
JOIN {context} ctx on rc.contextid = ctx.id
WHERE rc.contextid $incontexts AND rc.capability $incaps $excludelockedcaps";
$rcs = $DB->get_records_sql($sql, array_merge($cparams, $capsparams, $excludelockedcapsparams));
foreach ($rcs as $rc) {
$defs[$rc->path][$rc->roleid] = $rc->permission;
}
$access = array();
if (!empty($defs)) {
foreach ($contextpaths as $path) {
if (empty($defs[$path])) {
continue;
}
foreach ($defs[$path] as $roleid => $perm) {
if ($perm == CAP_PROHIBIT) {
$access[$roleid] = CAP_PROHIBIT;
continue;
}
if (!isset($access[$roleid])) {
$access[$roleid] = (int) $perm;
}
}
}
}
unset($defs);
// Make lists of roles that are needed and prohibited.
$needed = array(); // One of these is enough.
$prohibited = array(); // Must not have any of these.
foreach ($access as $roleid => $perm) {
if ($perm == CAP_PROHIBIT) {
unset($needed[$roleid]);
$prohibited[$roleid] = true;
} else {
if ($perm == CAP_ALLOW and empty($prohibited[$roleid])) {
$needed[$roleid] = true;
}
}
}
$defaultuserroleid = isset($CFG->defaultuserroleid) ? $CFG->defaultuserroleid : 0;
$defaultfrontpageroleid = isset($CFG->defaultfrontpageroleid) ? $CFG->defaultfrontpageroleid : 0;
$nobody = false;
if ($isfrontpage) {
if (!empty($prohibited[$defaultuserroleid]) or !empty($prohibited[$defaultfrontpageroleid])) {
$nobody = true;
} else {
if (!empty($needed[$defaultuserroleid]) or !empty($needed[$defaultfrontpageroleid])) {
// Everybody not having prohibit has the capability.
$needed = array();
} else {
if (empty($needed)) {
$nobody = true;
}
}
}
} else {
if (!empty($prohibited[$defaultuserroleid])) {
$nobody = true;
} else {
if (!empty($needed[$defaultuserroleid])) {
// Everybody not having prohibit has the capability.
$needed = array();
} else {
if (empty($needed)) {
$nobody = true;
}
}
}
}
if ($nobody) {
// Nobody can match so return some SQL that does not return any results.
$wheres[] = "1 = 2";
} else {
if ($needed) {
$ctxids = implode(',', $contextids);
$roleids = implode(',', array_keys($needed));
$joins[] = "JOIN {role_assignments} {$prefix}ra3
ON ({$prefix}ra3.userid = $useridcolumn
AND {$prefix}ra3.roleid IN ($roleids)
AND {$prefix}ra3.contextid IN ($ctxids))";
}
if ($prohibited) {
$ctxids = implode(',', $contextids);
$roleids = implode(',', array_keys($prohibited));
$joins[] = "LEFT JOIN {role_assignments} {$prefix}ra4
ON ({$prefix}ra4.userid = $useridcolumn
AND {$prefix}ra4.roleid IN ($roleids)
AND {$prefix}ra4.contextid IN ($ctxids))";
$wheres[] = "{$prefix}ra4.id IS NULL";
}
}
$wheres[] = "$useridcolumn <> :{$prefix}guestid";
$params["{$prefix}guestid"] = $CFG->siteguest;
$joins = implode("\n", $joins);
$wheres = "(" . implode(" AND ", $wheres) . ")";
return new \core\dml\sql_join($joins, $wheres, $params);
}

25
lib/classes/dml/sql_join.php
View File

@ -31,6 +31,14 @@ defined('MOODLE_INTERNAL') || die();
/**
* An object that contains sql join fragments.
*
* An example of how to use this class in a simple query, where you have got
* a join that is a join to the user table:
*
* $users = $DB->get_records_sql("SELECT u.*
* FROM {user} u
* {$sqljoin->joins}
* WHERE {$sqljoin->wheres}", $sqljoin->params);
*
* @since Moodle 3.1
* @package core
* @category dml
@ -54,16 +62,31 @@ class sql_join {
*/
public $params;
/**
* @var bool if true this join is guaranteed to never match any rows.
* In this case, the calling code may be able to completely
* skip doing the database query.
* @since Moodle 3.9/3.8.3/3.7.6.
*/
public $cannotmatchanyrows;
/**
* Create an object that contains sql join fragments.
*
* Note, even if you set $cannotmatchanyrows to true, it is
* important to also set the other fields because the calling
* code is not required to check it. For example
* new \core\dml\sql_join('', '1 = 2', [], true);
*
* @param string $joins The join sql fragment.
* @param string $wheres The where sql fragment.
* @param array $params Any parameter values.
* @param bool $cannotmatchanyrows If true, this join is guaranteed to match no rows. See comment on the field above.
*/
public function __construct($joins = '', $wheres = '', $params = array()) {
public function __construct($joins = '', $wheres = '', $params = array(), $cannotmatchanyrows = false) {
$this->joins = $joins;
$this->wheres = $wheres;
$this->params = $params;
$this->cannotmatchanyrows = $cannotmatchanyrows;
}
}

71
lib/tests/accesslib_test.php
View File

@ -3850,10 +3850,6 @@ class core_accesslib_testcase extends advanced_testcase {
$this->assertFalse(array_key_exists($guest->id, $users));
}
/**
* Test updating of role capabilities during upgrade
* @return void
*/
public function test_get_with_capability_sql() {
global $DB;
@ -3905,6 +3901,73 @@ class core_accesslib_testcase extends advanced_testcase {
$this->assertFalse(array_key_exists($guest->id, $users));
}
/**
* Get the test cases for {@link test_get_with_capability_join_when_overrides_present()}.
*
* The particular capabilties used here do not really matter. What is important is
* that they are capabilities which the Student roles has by default, but the
* authenticated suser role does not.
*
* @return array
*/
public function get_get_with_capability_join_override_cases() {
return [
'no overrides' => [true, []],
'one override' => [true, ['moodle/course:viewscales']],
'both overrides' => [false, ['moodle/course:viewscales', 'moodle/question:flag']],
];
}
/**
* Test get_with_capability_join.
*
* @dataProvider get_get_with_capability_join_override_cases
*
* @param bool $studentshouldbereturned whether, with this combination of capabilities, the student should be in the results.
* @param array $capabilitiestoprevent capabilities to override to prevent in the course context.
*/
public function test_get_with_capability_join_when_overrides_present(
bool $studentshouldbereturned, array $capabilitiestoprevent) {
global $DB;
$this->resetAfterTest();
$generator = $this->getDataGenerator();
// Create a course.
$category = $generator->create_category();
$course = $generator->create_course(['category' => $category->id]);
// Create a user.
$student = $generator->create_user();
$studentrole = $DB->get_record('role', ['shortname' => 'student'], '*', MUST_EXIST);
$generator->enrol_user($student->id, $course->id, $studentrole->id);
// This test assumes that by default the student roles has the two
// capabilities. Check this now in case the role definitions are every changed.
$coursecontext = context_course::instance($course->id);
$this->assertTrue(has_capability('moodle/course:viewscales', $coursecontext, $student));
$this->assertTrue(has_capability('moodle/question:flag', $coursecontext, $student));
// We test cases where there are a varying number of prevent overrides.
foreach ($capabilitiestoprevent as $capability) {
role_change_permission($studentrole->id, $coursecontext, $capability, CAP_PREVENT);
}
// So now, assemble our query using the method under test, and verify that it returns the student.
$sqljoin = get_with_capability_join($coursecontext,
['moodle/course:viewscales', 'moodle/question:flag'], 'u.id');
$users = $DB->get_records_sql("SELECT u.*
FROM {user} u
{$sqljoin->joins}
WHERE {$sqljoin->wheres}", $sqljoin->params);
if ($studentshouldbereturned) {
$this->assertEquals([$student->id], array_keys($users));
} else {
$this->assertEmpty($users);
}
}
/**
* Test the get_profile_roles() function.
*/