diff --git a/admin/tool/lp/tests/externallib_test.php b/admin/tool/lp/tests/externallib_test.php index 138a4204be7ab..44c95215f3150 100644 --- a/admin/tool/lp/tests/externallib_test.php +++ b/admin/tool/lp/tests/externallib_test.php @@ -116,7 +116,7 @@ protected function setUp() { $this->userrole = create_role('User role', 'lpuserrole', 'learning plan user role description'); assign_capability('moodle/competency:competencymanage', CAP_ALLOW, $this->creatorrole, $syscontext->id); - assign_capability('moodle/competency:competencycompetencyconfigure', CAP_ALLOW, $this->creatorrole, $syscontext->id); + assign_capability('moodle/competency:coursecompetencyconfigure', CAP_ALLOW, $this->creatorrole, $syscontext->id); assign_capability('moodle/competency:planmanage', CAP_ALLOW, $this->creatorrole, $syscontext->id); assign_capability('moodle/competency:planmanagedraft', CAP_ALLOW, $this->creatorrole, $syscontext->id); assign_capability('moodle/competency:planmanageown', CAP_ALLOW, $this->creatorrole, $syscontext->id); diff --git a/competency/tests/external_test.php b/competency/tests/external_test.php index 1a26d38198563..4c835413ac03a 100644 --- a/competency/tests/external_test.php +++ b/competency/tests/external_test.php @@ -139,7 +139,7 @@ protected function setUp() { $this->userrole = create_role('User role', 'userrole', 'learning plan user role description'); assign_capability('moodle/competency:competencymanage', CAP_ALLOW, $this->creatorrole, $syscontext->id); - assign_capability('moodle/competency:competencycompetencyconfigure', CAP_ALLOW, $this->creatorrole, $syscontext->id); + assign_capability('moodle/competency:coursecompetencyconfigure', CAP_ALLOW, $this->creatorrole, $syscontext->id); assign_capability('moodle/competency:competencyview', CAP_ALLOW, $this->userrole, $syscontext->id); assign_capability('moodle/competency:planmanage', CAP_ALLOW, $this->creatorrole, $syscontext->id); assign_capability('moodle/competency:planmanagedraft', CAP_ALLOW, $this->creatorrole, $syscontext->id); diff --git a/lib/accesslib.php b/lib/accesslib.php index d9970782eb248..f45bdee70991b 100644 --- a/lib/accesslib.php +++ b/lib/accesslib.php @@ -330,6 +330,7 @@ function get_role_definitions_uncached(array $roleids) { $sql = "SELECT ctx.path, rc.roleid, rc.capability, rc.permission FROM {role_capabilities} rc JOIN {context} ctx ON rc.contextid = ctx.id + JOIN {capabilities} cap ON rc.capability = cap.name WHERE rc.roleid $sql ORDER BY ctx.path, rc.roleid, rc.capability"; $rs = $DB->get_recordset_sql($sql, $params); @@ -1144,7 +1145,17 @@ function is_safe_capability($capability) { */ function get_local_override($roleid, $contextid, $capability) { global $DB; - return $DB->get_record('role_capabilities', array('roleid'=>$roleid, 'capability'=>$capability, 'contextid'=>$contextid)); + + return $DB->get_record_sql(" + SELECT rc.* + FROM {role_capabilities} rc + JOIN {capability} cap ON rc.capability = cap.name + WHERE rc.roleid = :roleid AND rc.capability = :capability AND rc.contextid = :contextid", [ + 'roleid' => $roleid, + 'contextid' => $contextid, + 'capability' => $capability, + + ]); } /** @@ -1290,6 +1301,11 @@ function assign_capability($capability, $permission, $roleid, $contextid, $overw $context = context::instance_by_id($contextid); } + // Capability must exist. + if (!$capinfo = get_capability_info($capability)) { + throw new coding_exception("Capability '{$capability}' was not found! This has to be fixed in code."); + } + if (empty($permission) || $permission == CAP_INHERIT) { // if permission is not set unassign_capability($capability, $roleid, $context->id); return true; @@ -1337,6 +1353,11 @@ function assign_capability($capability, $permission, $roleid, $contextid, $overw function unassign_capability($capability, $roleid, $contextid = null) { global $DB; + // Capability must exist. + if (!$capinfo = get_capability_info($capability)) { + throw new coding_exception("Capability '{$capability}' was not found! This has to be fixed in code."); + } + if (!empty($contextid)) { if ($contextid instanceof context) { $context = $contextid; @@ -1391,6 +1412,7 @@ function get_roles_with_capability($capability, $permission = null, $context = n FROM {role} r WHERE r.id IN (SELECT rc.roleid FROM {role_capabilities} rc + JOIN {capabilities} cap ON rc.capability = cap.name WHERE rc.capability = :capname $contextsql $permissionsql)"; @@ -2177,6 +2199,9 @@ function update_capabilities($component = 'moodle') { $DB->insert_record('capabilities', $capability, false); + // Flush the cached, as we have changed DB. + cache::make('core', 'capabilities')->delete('core_capabilities'); + if (isset($capdef['clonepermissionsfrom']) && in_array($capdef['clonepermissionsfrom'], $existingcaps)){ if ($rolecapabilities = $DB->get_records('role_capabilities', array('capability'=>$capdef['clonepermissionsfrom']))){ foreach ($rolecapabilities as $rolecapability){ @@ -2230,9 +2255,6 @@ function capabilities_cleanup($component, $newcapdef = null) { if (empty($newcapdef) || array_key_exists($cachedcap->name, $newcapdef) === false) { - // Remove from capabilities cache. - $DB->delete_records('capabilities', array('name'=>$cachedcap->name)); - $removedcount++; // Delete from roles. if ($roles = get_roles_with_capability($cachedcap->name)) { foreach($roles as $role) { @@ -2241,6 +2263,13 @@ function capabilities_cleanup($component, $newcapdef = null) { } } } + + // Remove from role_capabilities for any old ones. + $DB->delete_records('role_capabilities', array('capability' => $cachedcap->name)); + + // Remove from capabilities cache. + $DB->delete_records('capabilities', array('name' => $cachedcap->name)); + $removedcount++; } // End if. } } @@ -2305,10 +2334,12 @@ function role_context_capabilities($roleid, context $context, $cap = '') { } $sql = "SELECT rc.* - FROM {role_capabilities} rc, {context} c + FROM {role_capabilities} rc + JOIN {context} c ON rc.contextid = c.id + JOIN {capabilities} cap ON rc.capability = cap.name WHERE rc.contextid in $contexts AND rc.roleid = ? - AND rc.contextid = c.id $search + $search ORDER BY c.contextlevel DESC, rc.capability DESC"; $capabilities = array(); @@ -3024,6 +3055,7 @@ function get_switchable_roles(context $context) { SELECT r.id, r.name, r.shortname, rn.name AS coursealias FROM (SELECT DISTINCT rc.roleid FROM {role_capabilities} rc + $extrajoins $extrawhere) idlist JOIN {role} r ON r.id = idlist.roleid @@ -3286,6 +3318,7 @@ function get_users_by_capability(context $context, $capability, $fields = '', $s $params = array_merge($params, $params2); $sql = "SELECT rc.id, rc.roleid, rc.permission, rc.capability, 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"; @@ -4381,6 +4414,7 @@ function get_roles_with_cap_in_context($context, $capability) { $sql = "SELECT rc.id, rc.roleid, rc.permission, ctx.depth FROM {role_capabilities} rc JOIN {context} ctx ON ctx.id = rc.contextid + JOIN {capabilities} cap ON rc.capability = cap.name WHERE rc.capability = :cap AND ctx.id IN ($ctxids) ORDER BY rc.roleid ASC, ctx.depth DESC"; $params = array('cap'=>$capability); @@ -4500,6 +4534,7 @@ function prohibit_is_removable($roleid, context $context, $capability) { $sql = "SELECT ctx.id FROM {role_capabilities} rc JOIN {context} ctx ON ctx.id = rc.contextid + JOIN {capabilities} cap ON rc.capability = cap.name WHERE rc.roleid = :roleid AND rc.permission = :prohibit AND rc.capability = :cap AND ctx.id IN ($ctxids) ORDER BY ctx.depth DESC"; @@ -4543,6 +4578,7 @@ function role_change_permission($roleid, $context, $capname, $permission) { $sql = "SELECT ctx.id, rc.permission, ctx.depth FROM {role_capabilities} rc JOIN {context} ctx ON ctx.id = rc.contextid + JOIN {capabilities} cap ON rc.capability = cap.name WHERE rc.roleid = :roleid AND rc.capability = :cap AND ctx.id IN ($ctxids) ORDER BY ctx.depth DESC"; @@ -7224,6 +7260,7 @@ function get_with_capability_join(context $context, $capability, $useridcolumn) $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"; $rcs = $DB->get_records_sql($sql, array_merge($cparams, $capsparams)); diff --git a/lib/tests/accesslib_test.php b/lib/tests/accesslib_test.php index 3d7f0a04c0aea..96ffa17d6cdd0 100644 --- a/lib/tests/accesslib_test.php +++ b/lib/tests/accesslib_test.php @@ -1696,6 +1696,101 @@ public function test_has_capability_and_friends() { $this->assertFalse(has_all_capabilities($sca, $coursecontext, 0)); } + /** + * Test that assigning a fake cap does not return. + */ + public function test_fake_capability() { + global $DB; + + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(); + $coursecontext = context_course::instance($course->id); + $teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher'), '*', MUST_EXIST); + $teacher = $this->getDataGenerator()->create_user(); + + $fakecapname = 'moodle/fake:capability'; + + role_assign($teacherrole->id, $teacher->id, $coursecontext); + $admin = $DB->get_record('user', array('username' => 'admin')); + + // Test a capability which does not exist. + // Note: Do not use assign_capability because it will not allow fake caps. + $DB->insert_record('role_capabilities', (object) [ + 'contextid' => $coursecontext->id, + 'roleid' => $teacherrole->id, + 'capability' => $fakecapname, + 'permission' => CAP_ALLOW, + 'timemodified' => time(), + 'modifierid' => 0, + ]); + + // Check `has_capability`. + $this->assertFalse(has_capability($fakecapname, $coursecontext, $teacher)); + $this->assertDebuggingCalled("Capability \"{$fakecapname}\" was not found! This has to be fixed in code."); + $this->assertFalse(has_capability($fakecapname, $coursecontext, $admin)); + $this->assertDebuggingCalled("Capability \"{$fakecapname}\" was not found! This has to be fixed in code."); + + // Check `get_with_capability_sql` (with uses `get_with_capability_join`). + list($sql, $params) = get_with_capability_sql($coursecontext, $fakecapname); + $users = $DB->get_records_sql($sql, $params); + + $this->assertFalse(array_key_exists($teacher->id, $users)); + $this->assertFalse(array_key_exists($admin->id, $users)); + + // Check `get_users_by_capability`. + $users = get_users_by_capability($coursecontext, $fakecapname); + + $this->assertFalse(array_key_exists($teacher->id, $users)); + $this->assertFalse(array_key_exists($admin->id, $users)); + } + + /** + * Test that assigning a fake cap does not return. + */ + public function test_fake_capability_assign() { + global $DB; + + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(); + $coursecontext = context_course::instance($course->id); + $teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher'), '*', MUST_EXIST); + $teacher = $this->getDataGenerator()->create_user(); + + $capability = 'moodle/fake:capability'; + + role_assign($teacherrole->id, $teacher->id, $coursecontext); + $admin = $DB->get_record('user', array('username' => 'admin')); + + $this->expectException('coding_exception'); + $this->expectExceptionMessage("Capability '{$capability}' was not found! This has to be fixed in code."); + assign_capability($capability, CAP_ALLOW, $teacherrole->id, $coursecontext); + } + + /** + * Test that assigning a fake cap does not return. + */ + public function test_fake_capability_unassign() { + global $DB; + + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(); + $coursecontext = context_course::instance($course->id); + $teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher'), '*', MUST_EXIST); + $teacher = $this->getDataGenerator()->create_user(); + + $capability = 'moodle/fake:capability'; + + role_assign($teacherrole->id, $teacher->id, $coursecontext); + $admin = $DB->get_record('user', array('username' => 'admin')); + + $this->expectException('coding_exception'); + $this->expectExceptionMessage("Capability '{$capability}' was not found! This has to be fixed in code."); + unassign_capability($capability, CAP_ALLOW, $teacherrole->id, $coursecontext); + } + /** * Test that the caching in get_role_definitions() and get_role_definitions_uncached() * works as intended. @@ -2815,7 +2910,7 @@ public function test_permission_evaluation() { assign_capability('moodle/site:accessallgroups', CAP_ALLOW, $CFG->defaultfrontpageroleid, $frontpagepagecontext, true); assign_capability('mod/page:view', CAP_PREVENT, $allroles['guest'], $frontpagepagecontext, true); assign_capability('mod/page:view', CAP_ALLOW, $allroles['user'], $frontpagepagecontext, true); - assign_capability('moodle/page:view', CAP_ALLOW, $allroles['student'], $frontpagepagecontext, true); + assign_capability('mod/page:view', CAP_ALLOW, $allroles['student'], $frontpagepagecontext, true); assign_capability('moodle/site:accessallgroups', CAP_ALLOW, $CFG->defaultuserroleid, $frontpagecontext, true); assign_capability('moodle/site:accessallgroups', CAP_ALLOW, $CFG->defaultfrontpageroleid, $frontpagecontext, true); @@ -3443,10 +3538,10 @@ public function test_get_with_capability_sql() { $this->assertFalse(array_key_exists($guest->id, $users)); // Test role override. - assign_capability('moodle/site:backupcourse', CAP_PROHIBIT, $teacherrole->id, $coursecontext, true); - assign_capability('moodle/site:backupcourse', CAP_ALLOW, $studentrole->id, $coursecontext, true); + assign_capability('moodle/backup:backupcourse', CAP_PROHIBIT, $teacherrole->id, $coursecontext, true); + assign_capability('moodle/backup:backupcourse', CAP_ALLOW, $studentrole->id, $coursecontext, true); - list($sql, $params) = get_with_capability_sql($coursecontext, 'moodle/site:backupcourse'); + list($sql, $params) = get_with_capability_sql($coursecontext, 'moodle/backup:backupcourse'); $users = $DB->get_records_sql($sql, $params); $this->assertFalse(array_key_exists($teacher->id, $users)); diff --git a/mod/data/tests/externallib_test.php b/mod/data/tests/externallib_test.php index e8b8dc908e924..51ad2d449fe3e 100644 --- a/mod/data/tests/externallib_test.php +++ b/mod/data/tests/externallib_test.php @@ -274,7 +274,7 @@ public function test_view_database_not_enrolled_user() { public function test_view_database_no_capabilities() { // Test user with no capabilities. // We need a explicit prohibit since this capability is allowed for students by default. - assign_capability('mod/data:viewpage', CAP_PROHIBIT, $this->studentrole->id, $this->context->id); + assign_capability('mod/data:view', CAP_PROHIBIT, $this->studentrole->id, $this->context->id); accesslib_clear_all_caches_for_unit_testing(); $this->expectException('moodle_exception');