MDL-64971 access: Ensure that the capability exists when fetching

This commit is contained in:
Andrew Nicols 2019-02-28 08:39:20 +08:00
parent fec8550675
commit c40f6adbe0
5 changed files with 145 additions and 13 deletions

View File

@ -116,7 +116,7 @@ class tool_lp_external_testcase extends externallib_advanced_testcase {
$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);

View File

@ -139,7 +139,7 @@ class core_competency_external_testcase extends externallib_advanced_testcase {
$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);

View File

@ -349,6 +349,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";
$rs = $DB->get_recordset_sql($sql, $params);
@ -1191,7 +1192,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,
]);
}
/**
@ -1335,6 +1346,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;
@ -1380,6 +1396,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;
@ -1434,6 +1455,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)";
@ -2238,6 +2260,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){
@ -2291,9 +2316,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) {
@ -2302,6 +2324,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.
}
}
@ -2366,10 +2395,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();
@ -3108,6 +3139,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
@ -3422,6 +3454,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";
@ -4487,6 +4520,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);
@ -4606,6 +4640,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";
@ -4648,6 +4683,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";
@ -7524,6 +7560,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));

View File

@ -1804,6 +1804,101 @@ class core_accesslib_testcase extends advanced_testcase {
$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.
@ -2962,7 +3057,7 @@ class core_accesslib_testcase extends advanced_testcase {
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);
@ -3672,10 +3767,10 @@ class core_accesslib_testcase extends advanced_testcase {
$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));

View File

@ -274,7 +274,7 @@ class mod_data_external_testcase extends externallib_advanced_testcase {
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');