Skip to content

Commit

Permalink
accesslib: get_user_by_capability() - Move hidden RA checks to subselect
Browse files Browse the repository at this point in the history
we don't deal with RAs in the main SELECT -- we deal with _capabilities_
which is an entirely different matter ;-) -- so push the ra.hidden check
into the subselect.

Also, remove ra.hidden from the default list of fields. Hopefully no
callers are using ra.hidden -- if they are, they should be calling
something else, as this function deals with capabilities. So we might
need an audit of callers, to check that noone is expecting ra.hidden
to be there.

MDL-12452
  • Loading branch information
martinlanghoff committed Jan 6, 2008
1 parent a4436c6 commit d2c5b7a
Showing 1 changed file with 9 additions and 6 deletions.
15 changes: 9 additions & 6 deletions lib/accesslib.php
Original file line number Diff line number Diff line change
Expand Up @@ -4281,7 +4281,9 @@ function get_users_by_capability($context, $capability, $fields='', $sort='',

/// Set up hidden role-assignments sql
if ($view && !has_capability('moodle/role:viewhiddenassigns', $context)) {
$wherecond['hiddenra'] = ' ra.hidden = 0 ';
$condhiddenra = 'AND ra.hidden = 0 ';
} else {
$condhiddenra = '';
}

// Collect WHERE conditions
Expand All @@ -4293,9 +4295,9 @@ function get_users_by_capability($context, $capability, $fields='', $sort='',
/// Set up default fields
if (empty($fields)) {
if ($iscoursepage) {
$fields = 'u.*, ul.timeaccess as lastaccess, ra.hidden';
$fields = 'u.*, ul.timeaccess as lastaccess';
} else {
$fields = 'u.*, ra.hidden';
$fields = 'u.*';
}
}

Expand Down Expand Up @@ -4352,6 +4354,7 @@ function get_users_by_capability($context, $capability, $fields='', $sort='',
FROM {$CFG->prefix}role_assignments ssra
WHERE ssra.contextid IN ($ctxids)
AND ssra.roleid IN (".implode(',',$roleids) .")
$condhiddenra
) ra ON ra.userid = u.id
$uljoin ";
$where = " WHERE u.deleted = 0 ";
Expand Down Expand Up @@ -4407,15 +4410,14 @@ function get_users_by_capability($context, $capability, $fields='', $sort='',
// with a SELECT FROM user LEFT OUTER JOIN against ra -
// This is expensive on the SQL and PHP sides -
// moves a ton of data across the wire.

// TODO -- test!
$ss = "SELECT u.id as userid, ra.roleid,
ctx.depth
FROM {$CFG->prefix}user u
LEFT OUTER JOIN {$CFG->prefix}role_assignments ra
ON (ra.userid = u.id
AND ra.contextid IN ($ctxids)
AND ra.roleid IN (".implode(',',$roleids) ."))
AND ra.roleid IN (".implode(',',$roleids) .")
$condhiddenra)
LEFT OUTER JOIN {$CFG->prefix}context ctx
ON ra.contextid=ctx.id
WHERE u.deleted=0";
Expand All @@ -4428,6 +4430,7 @@ function get_users_by_capability($context, $capability, $fields='', $sort='',
JOIN {$CFG->prefix}context ctx
ON ra.contextid=ctx.id
WHERE ra.contextid IN ($ctxids)
$condhiddenra
AND ra.roleid IN (".implode(',',$roleids) .")";
}

Expand Down

0 comments on commit d2c5b7a

Please sign in to comment.