MDL-59977 core: do not directly check 'viewparticipant' capability
authorMark Nelson <markn@moodle.com>
Wed, 30 Aug 2017 07:11:20 +0000 (15:11 +0800)
committerMark Nelson <markn@moodle.com>
Mon, 11 Sep 2017 04:44:27 +0000 (12:44 +0800)
17 files changed:
blocks/activity_results/block_activity_results.php
blocks/participants/block_participants.php
course/lib.php
course/recent_form.php
course/tests/courselib_test.php
enrol/externallib.php
lib/classes/analytics/target/no_teaching.php
lib/navigationlib.php
lib/upgrade.txt
message/classes/api.php
mod/forum/lib.php
notes/delete.php
notes/edit.php
notes/index.php
user/externallib.php
user/index.php
user/messageselect.php

index de9b9ed..52fb8e9 100644 (file)
@@ -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 @@ class block_activity_results extends block_base {
                 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 = '';
index f8214df..b1b433b 100644 (file)
  * @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 @@ class block_participants extends block_list {
             $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;
             }
index 2ccc6f1..b38c641 100644 (file)
@@ -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', '');
+    }
+}
index cc2750a..d3056ea 100644 (file)
@@ -27,6 +27,7 @@ if (!defined('MOODLE_INTERNAL')) {
     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 @@ class recent_form extends moodleform {
         }
 
         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) {
index 702771f..6d53ba5 100644 (file)
@@ -3904,4 +3904,174 @@ class core_course_courselib_testcase extends advanced_testcase {
         // 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());
+    }
 }
index 370141a..d11bfab 100644 (file)
@@ -86,6 +86,8 @@ class core_enrol_external extends external_api {
      */
     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 @@ class core_enrol_external extends external_api {
                 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 @@ class core_enrol_external extends external_api {
      * @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 @@ class core_enrol_external extends external_api {
                 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 @@ class core_enrol_external extends external_api {
      */
     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 @@ class core_enrol_external extends external_api {
             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);
index 7d3714f..bacd39a 100644 (file)
@@ -63,6 +63,9 @@ class no_teaching extends \core_analytics\local\target\binary {
      * @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 @@ class no_teaching extends \core_analytics\local\target\binary {
         $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,
index afcd47e..2cbe2d7 100644 (file)
@@ -2212,6 +2212,8 @@ class global_navigation extends navigation_node {
     protected function load_for_user($user=null, $forceforcontext=false) {
         global $DB, $CFG, $USER, $SITE;
 
+        require_once($CFG->dirroot . '/course/lib.php');
+
         if ($user === null) {
             // We can't require login here but if the user isn't logged in we don't
             // want to show anything
@@ -2258,7 +2260,7 @@ class global_navigation extends navigation_node {
         } else if ($USER->id != $user->id) {
             // This is the site so add a users node to the root branch.
             $usersnode = $this->rootnodes['users'];
-            if (has_capability('moodle/course:viewparticipants', $coursecontext)) {
+            if (course_can_view_participants($coursecontext)) {
                 $usersnode->action = new moodle_url('/user/index.php', array('id' => $course->id));
             }
             $userviewurl = new moodle_url('/user/profile.php', $baseargs);
index cd4d742..38772ea 100644 (file)
@@ -44,6 +44,8 @@ information provided here is intended especially for developers.
 * user_can_view_profile() now also checks the moodle/user:viewalldetails capability.
 * The core/modal_confirm dialogue has been deprecated. Please use the core/modal_save_cancel dialogue instead. Please ensure you
   update to use the ModalEvents.save and ModalEvents.cancel events instead of their yes/no counterparts.
+* Instead of checking the 'moodle/course:viewparticipants' and 'moodle/site:viewparticipants' capabilities use the
+  new functions course_can_view_participants() and course_require_view_participants().
 
 === 3.3.1 ===
 
index 642353f..2065529 100644 (file)
@@ -199,6 +199,10 @@ class api {
         // Make sure to limit searches to enrolled courses.
         $enrolledcourses = enrol_get_my_courses(array('id', 'cacherev'));
         $courses = array();
+        // Really we want the user to be able to view the participants if they have the capability
+        // 'moodle/course:viewparticipants' or 'moodle/course:enrolreview', but since the search_courses function
+        // only takes required parameters we can't. However, the chance of a user having 'moodle/course:enrolreview' but
+        // *not* 'moodle/course:viewparticipants' are pretty much zero, so it is not worth addressing.
         if ($arrcourses = \coursecat::search_courses(array('search' => $search), array('limit' => $limitnum),
                 array('moodle/course:viewparticipants'))) {
             foreach ($arrcourses as $course) {
index 0f080a9..03ecffd 100644 (file)
@@ -5362,6 +5362,8 @@ function forum_print_latest_discussions($course, $forum, $maxdiscussions = -1, $
                                         $currentgroup = -1, $groupmode = -1, $page = -1, $perpage = 100, $cm = null) {
     global $CFG, $USER, $OUTPUT;
 
+    require_once($CFG->dirroot . '/course/lib.php');
+
     if (!$cm) {
         if (!$cm = get_coursemodule_from_instance('forum', $forum->id, $forum->course)) {
             print_error('invalidcoursemodule');
@@ -5498,7 +5500,7 @@ function forum_print_latest_discussions($course, $forum, $maxdiscussions = -1, $
         }
     }
 
-    $canviewparticipants = has_capability('moodle/course:viewparticipants',$context);
+    $canviewparticipants = course_can_view_participants($context);
     $canviewhiddentimedposts = has_capability('mod/forum:viewhiddentimedposts', $context);
 
     $strdatestring = get_string('strftimerecentfull');
index 3e0efc4..cb184b5 100644 (file)
@@ -16,6 +16,7 @@
 
 require_once('../config.php');
 require_once('lib.php');
+require_once($CFG->dirroot . '/course/lib.php');
 
 $noteid = required_param('id', PARAM_INT);
 
@@ -59,9 +60,7 @@ if (data_submitted() && confirm_sesskey()) {
 
     // Output HTML.
     $link = null;
-    if (has_capability('moodle/course:viewparticipants', $context)
-        || has_capability('moodle/site:viewparticipants', context_system::instance())) {
-
+    if (course_can_view_participants($context) || course_can_view_participants(context_system::instance())) {
         $link = new moodle_url('/user/index.php', array('id' => $course->id));
     }
     $PAGE->navbar->add(get_string('participants'), $link);
index 88a697f..7b97ecc 100644 (file)
@@ -17,6 +17,7 @@
 require_once('../config.php');
 require_once('lib.php');
 require_once('edit_form.php');
+require_once($CFG->dirroot . '/course/lib.php');
 
 $noteid = optional_param('id', 0, PARAM_INT);
 
@@ -95,9 +96,7 @@ if ($noteid) {
 
 // Output HTML.
 $link = null;
-if (has_capability('moodle/course:viewparticipants', $context)
-    || has_capability('moodle/site:viewparticipants', context_system::instance())) {
-
+if (course_can_view_participants($context) || course_can_view_participants(context_system::instance())) {
     $link = new moodle_url('/user/index.php', array('id' => $course->id));
 }
 $PAGE->navbar->add(get_string('participants'), $link);
index d9f9775..3c5cd28 100644 (file)
@@ -22,6 +22,7 @@
  */
 require_once('../config.php');
 require_once('lib.php');
+require_once($CFG->dirroot . '/course/lib.php');
 
 $courseid     = optional_param('course', SITEID, PARAM_INT);
 $userid       = optional_param('user', 0, PARAM_INT);
@@ -110,9 +111,7 @@ if ($userid && $course->id == SITEID) {
     $PAGE->set_context(context_course::instance($courseid));
 } else {
     $link = null;
-    if (has_capability('moodle/course:viewparticipants', $coursecontext)
-        || has_capability('moodle/site:viewparticipants', $systemcontext)) {
-
+    if (course_can_view_participants($coursecontext) || course_can_view_participants($systemcontext)) {
         $link = new moodle_url('/user/index.php', array('id' => $course->id));
     }
 }
index 40576ed..d8b24a3 100644 (file)
@@ -1330,6 +1330,7 @@ class core_user_external extends external_api {
     public static function view_user_list($courseid) {
         global $CFG;
         require_once($CFG->dirroot . "/user/lib.php");
+        require_once($CFG->dirroot . '/course/lib.php');
 
         $params = self::validate_parameters(self::view_user_list_parameters(),
                                             array(
@@ -1351,11 +1352,7 @@ class core_user_external extends external_api {
         }
         self::validate_context($context);
 
-        if ($course->id == SITEID) {
-            require_capability('moodle/site:viewparticipants', $context);
-        } else {
-            require_capability('moodle/course:viewparticipants', $context);
-        }
+        course_require_view_participants($context);
 
         user_list_view($course, $context);
 
index 9431d52..3b1961c 100644 (file)
@@ -74,16 +74,10 @@ $frontpagectx = context_course::instance(SITEID);
 
 if ($isfrontpage) {
     $PAGE->set_pagelayout('admin');
-    if (!has_any_capability(['moodle/site:viewparticipants', 'moodle/course:enrolreview'], $systemcontext)) {
-        // We know they do not have any of the capabilities, so lets throw an exception using the capability with the least access.
-        throw new required_capability_exception($systemcontext, 'moodle/site:viewparticipants', 'nopermissions', '');
-    }
+    course_require_view_participants($systemcontext);
 } else {
     $PAGE->set_pagelayout('incourse');
-    if (!has_any_capability(['moodle/course:viewparticipants', 'moodle/course:enrolreview'], $context)) {
-        // We know they do not have any of the capabilities, so lets throw an exception using the capability with the least access.
-        throw new required_capability_exception($context, 'moodle/course:viewparticipants', 'nopermissions', '');
-    }
+    course_require_view_participants($context);
 }
 
 // Trigger events.
index 99f2d96..9d7412d 100644 (file)
@@ -24,6 +24,7 @@
 
 require_once('../config.php');
 require_once($CFG->dirroot.'/message/lib.php');
+require_once($CFG->dirroot . '/course/lib.php');
 
 $id = required_param('id', PARAM_INT);
 $messagebody = optional_param('messagebody', '', PARAM_CLEANHTML);
@@ -115,8 +116,7 @@ if ($course->id == SITEID) {
 }
 
 $link = null;
-if (has_capability('moodle/course:viewparticipants', $coursecontext) ||
-    has_capability('moodle/site:viewparticipants', $systemcontext)) {
+if (course_can_view_participants($coursecontext) || course_can_view_participants($systemcontext)) {
     $link = new moodle_url("/user/index.php", array('id' => $course->id));
 }
 $PAGE->navbar->add(get_string('participants'), $link);