From 6f0c979f6e5d10743835f3b87387e77f5d2e8a31 Mon Sep 17 00:00:00 2001
From: Frederic Massart <fred@moodle.com>
Date: Tue, 22 Mar 2016 13:51:55 +0800
Subject: [PATCH] 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.
---
 admin/tool/lp/classes/api.php    | 12 ++++-----
 admin/tool/lp/tests/api_test.php | 44 ++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/admin/tool/lp/classes/api.php b/admin/tool/lp/classes/api.php
index 85661c8d119..9dcdcbcd728 100644
--- a/admin/tool/lp/classes/api.php
+++ b/admin/tool/lp/classes/api.php
@@ -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.
diff --git a/admin/tool/lp/tests/api_test.php b/admin/tool/lp/tests/api_test.php
index 63fb817274e..6896e9e1127 100644
--- a/admin/tool/lp/tests/api_test.php
+++ b/admin/tool/lp/tests/api_test.php
@@ -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();