MDL-53550 tool_lp: Relax capability checks on cohorts
Cohorts seem to rely on the calling code doing the right capability checks. In this case I applied the simple logic of checking if the cohort is visible, and if it is not then checking the capability. Methods using cohorts should apply relevant capability checks.
This commit is contained in:
@@ -2191,9 +2191,9 @@ class api {
|
||||
$cohort = $DB->get_record('cohort', array('id' => $cohort), '*', MUST_EXIST);
|
||||
}
|
||||
|
||||
// Check that the user can at least view this cohort.
|
||||
// Replicate logic in cohort_can_view_cohort() because we can't use it directly as we don't have a course context.
|
||||
$cohortcontext = context::instance_by_id($cohort->contextid);
|
||||
if (!has_any_capability(array('moodle/cohort:manage', 'moodle/cohort:view'), $cohortcontext)) {
|
||||
if (!$cohort->visible && !has_capability('moodle/cohort:view', $cohortcontext)) {
|
||||
throw new required_capability_exception($cohortcontext, 'moodle/cohort:view', 'nopermissions', '');
|
||||
}
|
||||
|
||||
@@ -2483,11 +2483,11 @@ class api {
|
||||
throw new coding_exception('A plan can not be created from a hidden template');
|
||||
}
|
||||
|
||||
// The user must be able to view the cohort.
|
||||
// Replicate logic in cohort_can_view_cohort() because we can't use it directly as we don't have a course context.
|
||||
$cohort = $DB->get_record('cohort', array('id' => $cohortid), '*', MUST_EXIST);
|
||||
$cohortctx = context::instance_by_id($cohort->contextid);
|
||||
if (!has_any_capability(array('moodle/cohort:manage', 'moodle/cohort:view'), $cohortctx)) {
|
||||
throw new required_capability_exception($cohortctx, 'tool/lp:templateread', 'nopermissions', '');
|
||||
$cohortcontext = context::instance_by_id($cohort->contextid);
|
||||
if (!$cohort->visible && !has_capability('moodle/cohort:view', $cohortcontext)) {
|
||||
throw new required_capability_exception($cohortcontext, 'moodle/cohort:view', 'nopermissions', '');
|
||||
}
|
||||
|
||||
// Convert the template to a plan.
|
||||
|
||||
@@ -1832,6 +1832,50 @@ class tool_lp_api_testcase extends advanced_testcase {
|
||||
$this->assertEquals(0, \tool_lp\template_cohort::count_records_select('templateid = :id', array('id' => $t2->get_id())));
|
||||
}
|
||||
|
||||
public function test_create_template_cohort_permissions() {
|
||||
$this->resetAfterTest(true);
|
||||
|
||||
$dg = $this->getDataGenerator();
|
||||
$lpg = $this->getDataGenerator()->get_plugin_generator('tool_lp');
|
||||
$cat = $dg->create_category();
|
||||
$catcontext = context_coursecat::instance($cat->id);
|
||||
$syscontext = context_system::instance();
|
||||
|
||||
$user = $dg->create_user();
|
||||
$role = $dg->create_role();
|
||||
assign_capability('tool/lp:templatemanage', CAP_ALLOW, $role, $syscontext->id, true);
|
||||
$dg->role_assign($role, $user->id, $syscontext->id);
|
||||
|
||||
$cohortrole = $dg->create_role();
|
||||
assign_capability('moodle/cohort:view', CAP_ALLOW, $cohortrole, $syscontext->id, true);
|
||||
|
||||
accesslib_clear_all_caches_for_unit_testing();
|
||||
|
||||
$c1 = $dg->create_cohort();
|
||||
$c2 = $dg->create_cohort(array('visible' => 0, 'contextid' => $catcontext->id));
|
||||
$t1 = $lpg->create_template();
|
||||
|
||||
$this->assertEquals(0, \tool_lp\template_cohort::count_records());
|
||||
|
||||
$this->setUser($user);
|
||||
$result = api::create_template_cohort($t1, $c1);
|
||||
$this->assertInstanceOf('tool_lp\\template_cohort', $result);
|
||||
|
||||
try {
|
||||
$result = api::create_template_cohort($t1, $c2);
|
||||
$this->fail('Permission required.');
|
||||
} catch(required_capability_exception $e) {
|
||||
// That's what should happen.
|
||||
}
|
||||
|
||||
// Try again with the right permissions.
|
||||
$dg->role_assign($cohortrole, $user->id, $catcontext->id);
|
||||
accesslib_clear_all_caches_for_unit_testing();
|
||||
|
||||
$result = api::create_template_cohort($t1, $c2);
|
||||
$this->assertInstanceOf('tool_lp\\template_cohort', $result);
|
||||
}
|
||||
|
||||
public function test_delete_template() {
|
||||
$this->resetAfterTest(true);
|
||||
$this->setAdminUser();
|
||||
|
||||
Reference in New Issue
Block a user