MDL-59977 core: do not directly check 'viewparticipant' capability

abgreeve · Sep 11, 2017 · 93b4771 · 93b4771
1 parent 32f9550
commit 93b4771
Show file tree

Hide file tree

Showing 17 changed files with 261 additions and 48 deletions.
diff --git a/blocks/activity_results/block_activity_results.php b/blocks/activity_results/block_activity_results.php
@@ -26,6 +26,7 @@
 defined('MOODLE_INTERNAL') || die();
 
 require_once($CFG->dirroot . '/lib/grade/constants.php');
+require_once($CFG->dirroot . '/course/lib.php');
 
 define('B_ACTIVITYRESULTS_NAME_FORMAT_FULL', 1);
 define('B_ACTIVITYRESULTS_NAME_FORMAT_ID',   2);
@@ -328,7 +329,7 @@ public function get_content() {
                 if ($nameformat == B_ACTIVITYRESULTS_NAME_FORMAT_FULL) {
                     if (has_capability('moodle/course:managegroups', $context)) {
                         $grouplink = $CFG->wwwroot.'/group/overview.php?id='.$courseid.'&amp;group=';
-                    } else if (has_capability('moodle/course:viewparticipants', $context)) {
+                    } else if (course_can_view_participants($context)) {
                         $grouplink = $CFG->wwwroot.'/user/index.php?id='.$courseid.'&amp;group=';
                     } else {
                         $grouplink = '';

diff --git a/blocks/participants/block_participants.php b/blocks/participants/block_participants.php
@@ -22,6 +22,17 @@
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
+defined('MOODLE_INTERNAL') || die();
+
+require_once($CFG->dirroot . '/course/lib.php');
+
+/**
+ * Participants block
+ *
+ * @package    block_participants
+ * @copyright  1999 onwards Martin Dougiamas (http://dougiamas.com)
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
 class block_participants extends block_list {
     function init() {
         $this->title = get_string('pluginname', 'block_participants');
@@ -48,12 +59,12 @@ function get_content() {
             $this->content = '';
             return $this->content;
         } else if ($this->page->course->id == SITEID) {
-            if (!has_capability('moodle/site:viewparticipants', context_system::instance())) {
+            if (!course_can_view_participants(context_system::instance())) {
                 $this->content = '';
                 return $this->content;
             }
         } else {
-            if (!has_capability('moodle/course:viewparticipants', $currentcontext)) {
+            if (!course_can_view_participants($currentcontext)) {
                 $this->content = '';
                 return $this->content;
             }

diff --git a/course/lib.php b/course/lib.php
@@ -3894,16 +3894,14 @@ function course_get_user_navigation_options($context, $course = null) {
     // Frontpage settings?
     if ($isfrontpage) {
         // We are on the front page, so make sure we use the proper capability (site:viewparticipants).
-        $options->participants = has_capability('moodle/site:viewparticipants', $sitecontext) ||
-            has_capability('moodle/course:enrolreview', $sitecontext);
+        $options->participants = course_can_view_participants($sitecontext);
         $options->badges = !empty($CFG->enablebadges) && has_capability('moodle/badges:viewbadges', $sitecontext);
         $options->tags = !empty($CFG->usetags) && $isloggedin;
         $options->search = !empty($CFG->enableglobalsearch) && has_capability('moodle/search:query', $sitecontext);
         $options->calendar = $isloggedin;
     } else {
         // We are in a course, so make sure we use the proper capability (course:viewparticipants).
-        $options->participants = has_capability('moodle/course:viewparticipants', $context) ||
-            has_capability('moodle/course:enrolreview', $context);
+        $options->participants = course_can_view_participants($context);
         $options->badges = !empty($CFG->enablebadges) && !empty($CFG->badges_allowcoursebadges) &&
                             has_capability('moodle/badges:viewbadges', $context);
         // Add view grade report is permitted.
@@ -4237,3 +4235,34 @@ function course_check_module_updates_since($cm, $from, $fileareas = array(), $fi
 
     return $updates;
 }
+
+/**
+ * Returns true if the user can view the participant page, false otherwise,
+ *
+ * @param context $context The context we are checking.
+ * @return bool
+ */
+function course_can_view_participants($context) {
+    $viewparticipantscap = 'moodle/course:viewparticipants';
+    if ($context->contextlevel == CONTEXT_SYSTEM) {
+        $viewparticipantscap = 'moodle/site:viewparticipants';
+    }
+
+    return has_any_capability([$viewparticipantscap, 'moodle/course:enrolreview'], $context);
+}
+
+/**
+ * Checks if a user can view the participant page, if not throws an exception.
+ *
+ * @param context $context The context we are checking.
+ * @throws required_capability_exception
+ */
+function course_require_view_participants($context) {
+    if (!course_can_view_participants($context)) {
+        $viewparticipantscap = 'moodle/course:viewparticipants';
+        if ($context->contextlevel == CONTEXT_SYSTEM) {
+            $viewparticipantscap = 'moodle/site:viewparticipants';
+        }
+        throw new required_capability_exception($context, $viewparticipantscap, 'nopermissions', '');
+    }
+}
diff --git a/course/recent_form.php b/course/recent_form.php
@@ -27,6 +27,7 @@
     die('Direct access to this script is forbidden.');    ///  It must be included from a Moodle page
 }
 
+require_once($CFG->dirroot . '/course/lib.php');
 require_once($CFG->libdir.'/formslib.php');
 
 class recent_form extends moodleform {
@@ -64,9 +65,9 @@ function definition() {
         }
 
         if ($COURSE->id == SITEID) {
-            $viewparticipants = has_capability('moodle/site:viewparticipants', context_system::instance());
+            $viewparticipants = course_can_view_participants(context_system::instance());
         } else {
-            $viewparticipants = has_capability('moodle/course:viewparticipants', $context);
+            $viewparticipants = course_can_view_participants($context);
         }
 
         if ($viewparticipants) {

diff --git a/course/tests/courselib_test.php b/course/tests/courselib_test.php
@@ -3904,4 +3904,174 @@ public function test_course_module_bulk_update_calendar_events() {
         // Update the assign instances for this course.
         $this->assertTrue(course_module_bulk_update_calendar_events('assign', $course->id));
     }
+
+    /**
+     * Test that a student can view participants in a course they are enrolled in.
+     */
+    public function test_course_can_view_participants_as_student() {
+        $this->resetAfterTest();
+
+        $course = $this->getDataGenerator()->create_course();
+        $coursecontext = context_course::instance($course->id);
+
+        $user = $this->getDataGenerator()->create_user();
+        $this->getDataGenerator()->enrol_user($user->id, $course->id);
+
+        $this->setUser($user);
+
+        $this->assertTrue(course_can_view_participants($coursecontext));
+    }
+
+    /**
+     * Test that a student in a course can not view participants on the site.
+     */
+    public function test_course_can_view_participants_as_student_on_site() {
+        $this->resetAfterTest();
+
+        $course = $this->getDataGenerator()->create_course();
+
+        $user = $this->getDataGenerator()->create_user();
+        $this->getDataGenerator()->enrol_user($user->id, $course->id);
+
+        $this->setUser($user);
+
+        $this->assertFalse(course_can_view_participants(context_system::instance()));
+    }
+
+    /**
+     * Test that an admin can view participants on the site.
+     */
+    public function test_course_can_view_participants_as_admin_on_site() {
+        $this->resetAfterTest();
+
+        $this->setAdminUser();
+
+        $this->assertTrue(course_can_view_participants(context_system::instance()));
+    }
+
+    /**
+     * Test teachers can view participants in a course they are enrolled in.
+     */
+    public function test_course_can_view_participants_as_teacher() {
+        global $DB;
+
+        $this->resetAfterTest();
+
+        $course = $this->getDataGenerator()->create_course();
+        $coursecontext = context_course::instance($course->id);
+
+        $user = $this->getDataGenerator()->create_user();
+        $roleid = $DB->get_field('role', 'id', array('shortname' => 'editingteacher'));
+        $this->getDataGenerator()->enrol_user($user->id, $course->id, $roleid);
+
+        $this->setUser($user);
+
+        $this->assertTrue(course_can_view_participants($coursecontext));
+    }
+
+    /**
+     * Check the teacher can still view the participants page without the 'viewparticipants' cap.
+     */
+    public function test_course_can_view_participants_as_teacher_without_view_participants_cap() {
+        global $DB;
+
+        $this->resetAfterTest();
+
+        $course = $this->getDataGenerator()->create_course();
+        $coursecontext = context_course::instance($course->id);
+
+        $user = $this->getDataGenerator()->create_user();
+        $roleid = $DB->get_field('role', 'id', array('shortname' => 'editingteacher'));
+        $this->getDataGenerator()->enrol_user($user->id, $course->id, $roleid);
+
+        $this->setUser($user);
+
+        // Disable one of the capabilties.
+        assign_capability('moodle/course:viewparticipants', CAP_PROHIBIT, $roleid, $coursecontext);
+
+        // Should still be able to view the page as they have the 'moodle/course:enrolreview' cap.
+        $this->assertTrue(course_can_view_participants($coursecontext));
+    }
+
+    /**
+     * Check the teacher can still view the participants page without the 'moodle/course:enrolreview' cap.
+     */
+    public function test_course_can_view_participants_as_teacher_without_enrol_review_cap() {
+        global $DB;
+
+        $this->resetAfterTest();
+
+        $course = $this->getDataGenerator()->create_course();
+        $coursecontext = context_course::instance($course->id);
+
+        $user = $this->getDataGenerator()->create_user();
+        $roleid = $DB->get_field('role', 'id', array('shortname' => 'editingteacher'));
+        $this->getDataGenerator()->enrol_user($user->id, $course->id, $roleid);
+
+        $this->setUser($user);
+
+        // Disable one of the capabilties.
+        assign_capability('moodle/course:enrolreview', CAP_PROHIBIT, $roleid, $coursecontext);
+
+        // Should still be able to view the page as they have the 'moodle/course:viewparticipants' cap.
+        $this->assertTrue(course_can_view_participants($coursecontext));
+    }
+
+    /**
+     * Check the teacher can not view the participants page without the required caps.
+     */
+    public function test_course_can_view_participants_as_teacher_without_required_caps() {
+        global $DB;
+
+        $this->resetAfterTest();
+
+        $course = $this->getDataGenerator()->create_course();
+        $coursecontext = context_course::instance($course->id);
+
+        $user = $this->getDataGenerator()->create_user();
+        $roleid = $DB->get_field('role', 'id', array('shortname' => 'editingteacher'));
+        $this->getDataGenerator()->enrol_user($user->id, $course->id, $roleid);
+
+        $this->setUser($user);
+
+        // Disable the capabilities.
+        assign_capability('moodle/course:viewparticipants', CAP_PROHIBIT, $roleid, $coursecontext);
+        assign_capability('moodle/course:enrolreview', CAP_PROHIBIT, $roleid, $coursecontext);
+
+        $this->assertFalse(course_can_view_participants($coursecontext));
+    }
+
+    /**
+     * Check that an exception is not thrown if we can view the participants page.
+     */
+    public function test_course_require_view_participants() {
+        $this->resetAfterTest();
+
+        $course = $this->getDataGenerator()->create_course();
+        $coursecontext = context_course::instance($course->id);
+
+        $user = $this->getDataGenerator()->create_user();
+        $this->getDataGenerator()->enrol_user($user->id, $course->id);
+
+        $this->setUser($user);
+
+        course_require_view_participants($coursecontext);
+    }
+
+    /**
+     * Check that an exception is thrown if we can't view the participants page.
+     */
+    public function test_course_require_view_participants_as_student_on_site() {
+        $this->resetAfterTest();
+
+        $course = $this->getDataGenerator()->create_course();
+
+        $user = $this->getDataGenerator()->create_user();
+        $this->getDataGenerator()->enrol_user($user->id, $course->id);
+
+        $this->setUser($user);
+
+        $this->expectException('required_capability_exception');
+        course_require_view_participants(context_system::instance());
+    }
 }
diff --git a/enrol/externallib.php b/enrol/externallib.php
@@ -86,6 +86,8 @@ public static function get_enrolled_users_with_capability_parameters() {
      */
     public static function get_enrolled_users_with_capability($coursecapabilities, $options) {
         global $CFG, $DB;
+
+        require_once($CFG->dirroot . '/course/lib.php');
         require_once($CFG->dirroot . "/user/lib.php");
 
         if (empty($coursecapabilities)) {
@@ -145,11 +147,8 @@ public static function get_enrolled_users_with_capability($coursecapabilities, $
                 throw new moodle_exception(get_string('errorcoursecontextnotvalid' , 'webservice', $exceptionparam));
             }
 
-            if ($courseid == SITEID) {
-                require_capability('moodle/site:viewparticipants', $context);
-            } else {
-                require_capability('moodle/course:viewparticipants', $context);
-            }
+            course_require_view_participants($context);
+
             // The accessallgroups capability is needed to use this option.
             if (!empty($groupid) && groups_is_member($groupid)) {
                 require_capability('moodle/site:accessallgroups', $coursecontext);
@@ -293,7 +292,9 @@ public static function get_users_courses_parameters() {
      * @return array of courses
      */
     public static function get_users_courses($userid) {
-        global $USER, $DB;
+        global $CFG, $USER, $DB;
+
+        require_once($CFG->dirroot . '/course/lib.php');
 
         // Do basic automatic PARAM checks on incoming data, using params description
         // If any problems are found then exceptions are thrown with helpful error messages
@@ -312,7 +313,7 @@ public static function get_users_courses($userid) {
                 continue;
             }
 
-            if ($userid != $USER->id and !has_capability('moodle/course:viewparticipants', $context)) {
+            if ($userid != $USER->id and !course_can_view_participants($context)) {
                 // we need capability to view participants
                 continue;
             }
@@ -520,6 +521,8 @@ public static function get_enrolled_users_parameters() {
      */
     public static function get_enrolled_users($courseid, $options = array()) {
         global $CFG, $USER, $DB;
+
+        require_once($CFG->dirroot . '/course/lib.php');
         require_once($CFG->dirroot . "/user/lib.php");
 
         $params = self::validate_parameters(
@@ -600,11 +603,8 @@ public static function get_enrolled_users($courseid, $options = array()) {
             throw new moodle_exception('errorcoursecontextnotvalid' , 'webservice', '', $exceptionparam);
         }
 
-        if ($courseid == SITEID) {
-            require_capability('moodle/site:viewparticipants', $context);
-        } else {
-            require_capability('moodle/course:viewparticipants', $context);
-        }
+        course_require_view_participants($context);
+
         // to overwrite this parameter, you need role:review capability
         if ($withcapability) {
             require_capability('moodle/role:review', $coursecontext);

diff --git a/lib/classes/analytics/target/no_teaching.php b/lib/classes/analytics/target/no_teaching.php
@@ -63,6 +63,9 @@ public static function get_name() : \lang_string {
      * @return \core_analytics\prediction_action[]
      */
     public function prediction_actions(\core_analytics\prediction $prediction, $includedetailsaction = false) {
+        global $CFG;
+
+        require_once($CFG->dirroot . '/course/lib.php');
 
         // No need to call the parent as the parent's action is view details and this target only have 1 feature.
         $actions = array();
@@ -75,7 +78,7 @@ public function prediction_actions(\core_analytics\prediction $prediction, $incl
         $actions['viewcourse'] = new \core_analytics\prediction_action('viewcourse', $prediction,
             $url, $pix, get_string('view'));
 
-        if (has_any_capability(['moodle/course:viewparticipants', 'moodle/course:enrolreview'], $sampledata['context'])) {
+        if (course_can_view_participants($sampledata['context'])) {
             $url = new \moodle_url('/user/index.php', array('id' => $course->id));
             $pix = new \pix_icon('i/cohort', get_string('participants'));
             $actions['viewparticipants'] = new \core_analytics\prediction_action('viewparticipants', $prediction,