Details
-
Type:
Bug
-
Status: Closed
-
Priority:
Critical
-
Resolution: Fixed
-
Affects Version/s: 1.9.4
-
Fix Version/s: 2.0
-
Component/s: Roles / Access
-
Labels:None
-
Affected Branches:MOODLE_19_STABLE
-
Fixed Branches:MOODLE_20_STABLE
-
Rank:31734
Description
Sam reported in MDL-19038:
Tim - the function get_u_b_c doesn't actually return all half-million users - but when you make query at CONTEXT_SYSTEM level like this, it really DOES retrieve all half-million users into php, then it filters them. This is about line 4670 in accesslib. Code below.
I think your lazy generation patch is correct (although it would be nice if somebody could make get_u_b_c faster for this case, but that's likely to be an awful lot harder to figure out).
if ($context->contextlevel == CONTEXT_SYSTEM || $isfrontpage || $defaultroleinteresting) { // Handle system / sitecourse / defaultrole-with-perhaps-neg-overrides // 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. $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) .") $condhiddenra) LEFT OUTER JOIN {$CFG->prefix}context ctx ON ra.contextid=ctx.id WHERE u.deleted=0";
Clearly this is silly. The CONTEXT_SYSTEM case must be easy to do in pure SQL.
Note that there are quite extensive unit tests for get_users_by_capability in HEAD. If you plan to work on this, you probably should use them to reduce the chance of regressions.
Activity
hmm, I already proposed simplified capability evaluation rules that eliminate this and similar problems completely...
postponing, sorry - changes in this area will require longer testing and review 
Anything we can do in 1.9.6? Can you update the status please?
sure, going to commit later today
What are you going to commit? I am happy to review patches.
grrrrrrrrrrrrrrrrrrrr, wrong bug! I was working on MDL-19004,
sorry!
The problem is caused by negative permissions in authenticated users role (negative perm in frontpage role does something similar) - that is not recommended at present and will not work much any way. Please do not use any negative capabilities in the definition of roles at all, in fact do not use it anywhere if possible.
Theoretically it should be possible to write such SQL at the system level, but I think it is a waste of time because the workaround is simple and in 2.0 the permissions where negative caps are involved will be probably different.
Changing fix version for now, sorry.
this should be finally fixed after the recent commit enrolment related changes, please note the activity code may need to be updated
thanks for the report
Petr
Adding a handful of watchers.