diff --git a/lib/classes/access/get_user_capability_course_helper.php b/lib/classes/access/get_user_capability_course_helper.php index 80b5b4578c8..93dccfa5a99 100644 --- a/lib/classes/access/get_user_capability_course_helper.php +++ b/lib/classes/access/get_user_capability_course_helper.php @@ -39,7 +39,10 @@ class get_user_capability_course_helper { * 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. + * effect) and then by role id. Note, however, that the resulting array only has + * the information that will be needed later. If there are Prohibits present in some + * roles, then they cannot be overridden by other roles or role overrides in lower contexts, + * therefore, such information, if any, is absent from the results. * * @param int $userid User id * @param string $capability Capability e.g. 'moodle/course:view' @@ -58,42 +61,101 @@ class get_user_capability_course_helper { } $rdefs = get_role_definitions(array_keys($roleids)); + // A prohibit in any relevant role prevents the capability + // in that context and all subcontexts. We need to track that. + // Here, the array keys are the paths where there is a prohibit the values are the role id. + $prohibitpaths = []; + // 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 ($accessdata['ra'] as $rapath => $roles) { + foreach ($roles as $roleid) { // Get role definition for that role. - foreach ($rdefs[$roleid] as $rolepath => $caps) { + foreach ($rdefs[$roleid] as $rdefpath => $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; + // Check a role definition or override above ra. + if (self::path_is_above($rdefpath, $rapath)) { + // Note that $rdefs is sorted by path, so if a more specific override + // exists, it will be processed later and override this one. + $effectivepath = $rapath; + } else if (self::path_is_above($rapath, $rdefpath)) { + $effectivepath = $rdefpath; } else { // Not inside an area where the user has the role, so ignore. continue; } + // Check for already seen prohibits in higher context. Overrides can't change that. + if (self::any_path_is_above($prohibitpaths, $effectivepath)) { + continue; + } + + // This is a releavant role assignment / permission combination. Save it. if (!array_key_exists($effectivepath, $pathroleperms)) { $pathroleperms[$effectivepath] = []; } $pathroleperms[$effectivepath][$roleid] = $caps[$capability]; + + // Update $prohibitpaths if necessary. + if ($caps[$capability] == CAP_PROHIBIT) { + // First remove any lower-context prohibits that might have come from other roles. + foreach ($prohibitpaths as $otherprohibitpath => $notused) { + if (self::path_is_above($effectivepath, $otherprohibitpath)) { + unset($prohibitpaths[$otherprohibitpath]); + } + } + $prohibitpaths[$effectivepath] = $roleid; + } } } } + // Finally, if a later role had a higher-level prohibit that an earlier role, + // there may be more bits we can prune - but don't prune the prohibits! + foreach ($pathroleperms as $effectivepath => $roleperms) { + if ($roleid = self::any_path_is_above($prohibitpaths, $effectivepath)) { + unset($pathroleperms[$effectivepath]); + $pathroleperms[$effectivepath][$roleid] = CAP_PROHIBIT; + } + } + return $pathroleperms; } + /** + * Test if a context path $otherpath is the same as, or underneath, $parentpath. + * + * @param string $parentpath the path of the parent context. + * @param string $otherpath the path of another context. + * @return bool true if $otherpath is underneath (or equal to) $parentpath. + */ + protected static function path_is_above($parentpath, $otherpath) { + return preg_match('~^' . $parentpath . '($|/)~', $otherpath); + } + + /** + * Test if a context path $otherpath is the same as, or underneath, any of $prohibitpaths. + * + * @param array $prohibitpaths array keys are context paths. + * @param string $otherpath the path of another context. + * @return int releavant $roleid if $otherpath is underneath (or equal to) + * any of the $prohibitpaths, 0 otherwise (so, can be used as a bool). + */ + protected static function any_path_is_above($prohibitpaths, $otherpath) { + foreach ($prohibitpaths as $prohibitpath => $roleid) { + if (self::path_is_above($prohibitpath, $otherpath)) { + return $roleid; + } + } + return 0; + } + /** * Calculates a permission tree based on an array of information about role permissions. * diff --git a/lib/tests/accesslib_test.php b/lib/tests/accesslib_test.php index 9e27b4b86d3..a4d16e5d60b 100644 --- a/lib/tests/accesslib_test.php +++ b/lib/tests/accesslib_test.php @@ -1707,6 +1707,25 @@ class core_accesslib_testcase extends advanced_testcase { $generator = $this->getDataGenerator(); $cap = 'moodle/course:view'; + // The structure being created here is this: + // + // All tests work with the single capability 'moodle/course:view'. + // + // ROLE DEF/OVERRIDE ROLE ASSIGNS + // Role: Allow Prohib Empty Def user u1 u2 u3 u4 u5 u6 u7 u8 + // System ALLOW PROHIBIT A E A+E + // cat1 ALLOW + // C1 (ALLOW) P + // C2 ALLOW E P + // cat2 PREVENT + // C3 ALLOW E + // C4 + // Misc. A + // C5 PREVENT A + // C6 PROHIBIT + // + // Front-page and guest role stuff from the end of this test not included in the diagram. + // Create a role which allows course:view and one that prohibits it, and one neither. $allowroleid = $generator->create_role(); $prohibitroleid = $generator->create_role(); @@ -1720,6 +1739,7 @@ class core_accesslib_testcase extends advanced_testcase { $cat2 = $generator->create_category(['parent' => $cat1->id]); // Create six courses - two in cat1, two in cat2, and two in default category. + // Shortnames are used for a sorting test. Otherwise they are not significant. $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']); @@ -1741,6 +1761,8 @@ class core_accesslib_testcase extends advanced_testcase { context_course::instance($c6->id)->id); assign_capability($cap, CAP_ALLOW, $emptyroleid, context_course::instance($c3->id)->id); + assign_capability($cap, CAP_ALLOW, $prohibitroleid, + context_course::instance($c2->id)->id); // User 1 has no roles except default user role. $u1 = $generator->create_user(); @@ -1800,6 +1822,22 @@ class core_accesslib_testcase extends advanced_testcase { $courses = get_user_capability_course($cap, $u6->id, true, '', 'id'); $this->assert_course_ids([$c3->id], $courses); + // User 7 has empty role in C2. + $u7 = $generator->create_user(); + role_assign($emptyroleid, $u7->id, context_course::instance($c2->id)->id); + + // Should get C1 by the default user role override, and C2 by the cat1 level override. + $courses = get_user_capability_course($cap, $u7->id, true, '', 'id'); + $this->assert_course_ids([$c1->id, $c2->id], $courses); + + // User 8 has prohibit role as system context, to verify that prohibits can't be overridden. + $u8 = $generator->create_user(); + role_assign($prohibitroleid, $u8->id, context_course::instance($c2->id)->id); + + // Should get C1 by the default user role override, no other courses because the prohibit cannot be overridden. + $courses = get_user_capability_course($cap, $u8->id, true, '', 'id'); + $this->assert_course_ids([$c1->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],