MDL-50173 ratings: Use proper checks to ensure ratings are viewable.

Mainly to verify groups visibility this new callback has been created.

Note this was originally 3 commits but for amending purposes they have
been squashed.

mod/data/lib.php

@@ -1524,6 +1524,53 @@ function data_rating_validate($params) {
     return true;
 }
 
+/**
+ * Can the current user see ratings for a given itemid?
+ *
+ * @param array $params submitted data
+ *            contextid => int contextid [required]
+ *            component => The component for this module - should always be mod_data [required]
+ *            ratingarea => object the context in which the rated items exists [required]
+ *            itemid => int the ID of the object being rated [required]
+ *            scaleid => int scale id [optional]
+ * @return bool
+ * @throws coding_exception
+ * @throws rating_exception
+ */
+function mod_data_rating_can_see_item_ratings($params) {
+    global $DB;
+
+    // Check the component is mod_data.
+    if (!isset($params['component']) || $params['component'] != 'mod_data') {
+        throw new rating_exception('invalidcomponent');
+    }
+
+    // Check the ratingarea is entry (the only rating area in data).
+    if (!isset($params['ratingarea']) || $params['ratingarea'] != 'entry') {
+        throw new rating_exception('invalidratingarea');
+    }
+
+    if (!isset($params['itemid'])) {
+        throw new rating_exception('invaliditemid');
+    }
+
+    $datasql = "SELECT d.id as dataid, d.course, r.groupid
+                  FROM {data_records} r
+                  JOIN {data} d ON r.dataid = d.id
+                 WHERE r.id = :itemid";
+    $dataparams = array('itemid' => $params['itemid']);
+    if (!$info = $DB->get_record_sql($datasql, $dataparams)) {
+        // Item doesn't exist.
+        throw new rating_exception('invaliditemid');
+    }
+
+    $course = $DB->get_record('course', array('id' => $info->course), '*', MUST_EXIST);
+    $cm = get_coursemodule_from_instance('data', $info->dataid, $course->id, false, MUST_EXIST);
+
+    // Make sure groups allow this user to see the item they're rating.
+    return groups_group_visible($info->groupid, $course, $cm);
+}
+
 
 /**
  * function that takes in the current data, number of items per page,

mod/data/tests/lib_test.php

@@ -35,9 +35,9 @@ require_once($CFG->dirroot . '/mod/data/lib.php');
  * @copyright  2013 Adrian Greeve
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-class data_lib_testcase extends advanced_testcase {
+class mod_data_lib_testcase extends advanced_testcase {
 
-    function test_data_delete_record() {
+    public function test_data_delete_record() {
         global $DB;
 
         $this->resetAfterTest();
@@ -231,4 +231,103 @@ class data_lib_testcase extends advanced_testcase {
         $this->assertEquals($url, $event->get_url());
         $this->assertEventContextNotUsed($event);
     }
+
+    /**
+     * Tests for mod_data_rating_can_see_item_ratings().
+     *
+     * @throws coding_exception
+     * @throws rating_exception
+     */
+    public function test_mod_data_rating_can_see_item_ratings() {
+        global $DB;
+
+        $this->resetAfterTest();
+
+        // Setup test data.
+        $course = new stdClass();
+        $course->groupmode = SEPARATEGROUPS;
+        $course->groupmodeforce = true;
+        $course = $this->getDataGenerator()->create_course($course);
+        $data = $this->getDataGenerator()->create_module('data', array('course' => $course->id));
+        $cm = get_coursemodule_from_instance('data', $data->id);
+        $context = context_module::instance($cm->id);
+
+        // Create users.
+        $user1 = $this->getDataGenerator()->create_user();
+        $user2 = $this->getDataGenerator()->create_user();
+        $user3 = $this->getDataGenerator()->create_user();
+        $user4 = $this->getDataGenerator()->create_user();
+
+        // Groups and stuff.
+        $role = $DB->get_record('role', array('shortname' => 'teacher'), '*', MUST_EXIST);
+        $this->getDataGenerator()->enrol_user($user1->id, $course->id, $role->id);
+        $this->getDataGenerator()->enrol_user($user2->id, $course->id, $role->id);
+        $this->getDataGenerator()->enrol_user($user3->id, $course->id, $role->id);
+        $this->getDataGenerator()->enrol_user($user4->id, $course->id, $role->id);
+
+        $group1 = $this->getDataGenerator()->create_group(array('courseid' => $course->id));
+        $group2 = $this->getDataGenerator()->create_group(array('courseid' => $course->id));
+        groups_add_member($group1, $user1);
+        groups_add_member($group1, $user2);
+        groups_add_member($group2, $user3);
+        groups_add_member($group2, $user4);
+
+        // Add data.
+        $field = data_get_field_new('text', $data);
+
+        $fielddetail = new stdClass();
+        $fielddetail->name = 'Name';
+        $fielddetail->description = 'Some name';
+
+        $field->define_field($fielddetail);
+        $field->insert_field();
+        $recordid = data_add_record($data, $group1->id);
+
+        $datacontent = array();
+        $datacontent['fieldid'] = $field->field->id;
+        $datacontent['recordid'] = $recordid;
+        $datacontent['content'] = 'Asterix';
+        $DB->insert_record('data_content', $datacontent);
+
+        // Now try to access it as various users.
+        unassign_capability('moodle/site:accessallgroups', $role->id);
+        $params = array('contextid' => 2,
+                        'component' => 'mod_data',
+                        'ratingarea' => 'entry',
+                        'itemid' => $recordid,
+                        'scaleid' => 2);
+        $this->setUser($user1);
+        $this->assertTrue(mod_data_rating_can_see_item_ratings($params));
+        $this->setUser($user2);
+        $this->assertTrue(mod_data_rating_can_see_item_ratings($params));
+        $this->setUser($user3);
+        $this->assertFalse(mod_data_rating_can_see_item_ratings($params));
+        $this->setUser($user4);
+        $this->assertFalse(mod_data_rating_can_see_item_ratings($params));
+
+        // Now try with accessallgroups cap and make sure everything is visible.
+        assign_capability('moodle/site:accessallgroups', CAP_ALLOW, $role->id, $context->id);
+        $this->setUser($user1);
+        $this->assertTrue(mod_data_rating_can_see_item_ratings($params));
+        $this->setUser($user2);
+        $this->assertTrue(mod_data_rating_can_see_item_ratings($params));
+        $this->setUser($user3);
+        $this->assertTrue(mod_data_rating_can_see_item_ratings($params));
+        $this->setUser($user4);
+        $this->assertTrue(mod_data_rating_can_see_item_ratings($params));
+
+        // Change group mode and verify visibility.
+        $course->groupmode = VISIBLEGROUPS;
+        $DB->update_record('course', $course);
+        unassign_capability('moodle/site:accessallgroups', $role->id);
+        $this->setUser($user1);
+        $this->assertTrue(mod_data_rating_can_see_item_ratings($params));
+        $this->setUser($user2);
+        $this->assertTrue(mod_data_rating_can_see_item_ratings($params));
+        $this->setUser($user3);
+        $this->assertTrue(mod_data_rating_can_see_item_ratings($params));
+        $this->setUser($user4);
+        $this->assertTrue(mod_data_rating_can_see_item_ratings($params));
+
+    }
 }

mod/forum/lib.php

@@ -3664,6 +3664,48 @@ function forum_rating_validate($params) {
     return true;
 }
 
+/**
+ * Can the current user see ratings for a given itemid?
+ *
+ * @param array $params submitted data
+ *            contextid => int contextid [required]
+ *            component => The component for this module - should always be mod_forum [required]
+ *            ratingarea => object the context in which the rated items exists [required]
+ *            itemid => int the ID of the object being rated [required]
+ *            scaleid => int scale id [optional]
+ * @return bool
+ * @throws coding_exception
+ * @throws rating_exception
+ */
+function mod_forum_rating_can_see_item_ratings($params) {
+    global $DB, $USER;
+
+    // Check the component is mod_forum.
+    if (!isset($params['component']) || $params['component'] != 'mod_forum') {
+        throw new rating_exception('invalidcomponent');
+    }
+
+    // Check the ratingarea is post (the only rating area in forum).
+    if (!isset($params['ratingarea']) || $params['ratingarea'] != 'post') {
+        throw new rating_exception('invalidratingarea');
+    }
+
+    if (!isset($params['itemid'])) {
+        throw new rating_exception('invaliditemid');
+    }
+
+    $post = $DB->get_record('forum_posts', array('id' => $params['itemid']), '*', MUST_EXIST);
+    $discussion = $DB->get_record('forum_discussions', array('id' => $post->discussion), '*', MUST_EXIST);
+    $forum = $DB->get_record('forum', array('id' => $discussion->forum), '*', MUST_EXIST);
+    $course = $DB->get_record('course', array('id' => $forum->course), '*', MUST_EXIST);
+    $cm = get_coursemodule_from_instance('forum', $forum->id, $course->id , false, MUST_EXIST);
+
+    // Perform some final capability checks.
+    if (!forum_user_can_see_post($forum, $discussion, $post, $USER, $cm)) {
+        return false;
+    }
+    return true;
+}
 
 /**
  * This function prints the overview of a discussion in the forum listing.

mod/forum/tests/lib_test.php

@@ -26,6 +26,7 @@ defined('MOODLE_INTERNAL') || die();
 
 global $CFG;
 require_once($CFG->dirroot . '/mod/forum/lib.php');
+require_once($CFG->dirroot . '/rating/lib.php');
 
 class mod_forum_lib_testcase extends advanced_testcase {
 
@@ -1394,4 +1395,107 @@ class mod_forum_lib_testcase extends advanced_testcase {
         return $discussion;
     }
 
+    /**
+     * Tests for mod_forum_rating_can_see_item_ratings().
+     *
+     * @throws coding_exception
+     * @throws rating_exception
+     */
+    public function test_mod_forum_rating_can_see_item_ratings() {
+        global $DB;
+
+        $this->resetAfterTest();
+
+        // Setup test data.
+        $course = new stdClass();
+        $course->groupmode = SEPARATEGROUPS;
+        $course->groupmodeforce = true;
+        $course = $this->getDataGenerator()->create_course($course);
+        $forum = $this->getDataGenerator()->create_module('forum', array('course' => $course->id));
+        $generator = self::getDataGenerator()->get_plugin_generator('mod_forum');
+        $cm = get_coursemodule_from_instance('forum', $forum->id);
+        $context = context_module::instance($cm->id);
+
+        // Create users.
+        $user1 = $this->getDataGenerator()->create_user();
+        $user2 = $this->getDataGenerator()->create_user();
+        $user3 = $this->getDataGenerator()->create_user();
+        $user4 = $this->getDataGenerator()->create_user();
+
+        // Groups and stuff.
+        $role = $DB->get_record('role', array('shortname' => 'teacher'), '*', MUST_EXIST);
+        $this->getDataGenerator()->enrol_user($user1->id, $course->id, $role->id);
+        $this->getDataGenerator()->enrol_user($user2->id, $course->id, $role->id);
+        $this->getDataGenerator()->enrol_user($user3->id, $course->id, $role->id);
+        $this->getDataGenerator()->enrol_user($user4->id, $course->id, $role->id);
+
+        $group1 = $this->getDataGenerator()->create_group(array('courseid' => $course->id));
+        $group2 = $this->getDataGenerator()->create_group(array('courseid' => $course->id));
+        groups_add_member($group1, $user1);
+        groups_add_member($group1, $user2);
+        groups_add_member($group2, $user3);
+        groups_add_member($group2, $user4);
+
+        $record = new stdClass();
+        $record->course = $forum->course;
+        $record->forum = $forum->id;
+        $record->userid = $user1->id;
+        $record->groupid = $group1->id;
+        $discussion = $generator->create_discussion($record);
+
+        // Retrieve the first post.
+        $post = $DB->get_record('forum_posts', array('discussion' => $discussion->id));
+
+        $ratingoptions = new stdClass;
+        $ratingoptions->context = $context;
+        $ratingoptions->ratingarea = 'post';
+        $ratingoptions->component = 'mod_forum';
+        $ratingoptions->itemid  = $post->id;
+        $ratingoptions->scaleid = 2;
+        $ratingoptions->userid  = $user2->id;
+        $rating = new rating($ratingoptions);
+        $rating->update_rating(2);
+
+        // Now try to access it as various users.
+        unassign_capability('moodle/site:accessallgroups', $role->id);
+        $params = array('contextid' => 2,
+                        'component' => 'mod_forum',
+                        'ratingarea' => 'post',
+                        'itemid' => $post->id,
+                        'scaleid' => 2);
+        $this->setUser($user1);
+        $this->assertTrue(mod_forum_rating_can_see_item_ratings($params));
+        $this->setUser($user2);
+        $this->assertTrue(mod_forum_rating_can_see_item_ratings($params));
+        $this->setUser($user3);
+        $this->assertFalse(mod_forum_rating_can_see_item_ratings($params));
+        $this->setUser($user4);
+        $this->assertFalse(mod_forum_rating_can_see_item_ratings($params));
+
+        // Now try with accessallgroups cap and make sure everything is visible.
+        assign_capability('moodle/site:accessallgroups', CAP_ALLOW, $role->id, $context->id);
+        $this->setUser($user1);
+        $this->assertTrue(mod_forum_rating_can_see_item_ratings($params));
+        $this->setUser($user2);
+        $this->assertTrue(mod_forum_rating_can_see_item_ratings($params));
+        $this->setUser($user3);
+        $this->assertTrue(mod_forum_rating_can_see_item_ratings($params));
+        $this->setUser($user4);
+        $this->assertTrue(mod_forum_rating_can_see_item_ratings($params));
+
+        // Change group mode and verify visibility.
+        $course->groupmode = VISIBLEGROUPS;
+        $DB->update_record('course', $course);
+        unassign_capability('moodle/site:accessallgroups', $role->id);
+        $this->setUser($user1);
+        $this->assertTrue(mod_forum_rating_can_see_item_ratings($params));
+        $this->setUser($user2);
+        $this->assertTrue(mod_forum_rating_can_see_item_ratings($params));
+        $this->setUser($user3);
+        $this->assertTrue(mod_forum_rating_can_see_item_ratings($params));
+        $this->setUser($user4);
+        $this->assertTrue(mod_forum_rating_can_see_item_ratings($params));
+
+    }
+
 }

mod/upgrade.txt

@@ -8,6 +8,8 @@ information provided here is intended especially for developers.
   https://docs.moodle.org/dev/version.php for details (MDL-43896).
 * Function scorm_view_display was renamed to scorm_print_launch to avoid
   confussion with new function scorm_view.
+* Modules using rating component must implement a callback mod_x_rating_can_see_item_ratings(). Refer
+  to mod_forum_rating_can_see_item_ratings() for example.
 
 === 2.9 ===
 

rating/classes/external.php

@@ -96,7 +96,13 @@ class core_rating_external extends external_api {
         self::validate_context($context);
 
         // Minimal capability required.
-        if (!has_capability('moodle/rating:view', $context)) {
+        $callbackparams = array('contextid' => $context->id,
+                        'component' => $component,
+                        'ratingarea' => $ratingarea,
+                        'itemid' => $itemid,
+                        'scaleid' => $scaleid);
+        if (!has_capability('moodle/rating:view', $context) ||
+                !component_callback($component, 'rating_can_see_item_ratings', array($callbackparams), true)) {
             throw new moodle_exception('noviewrate', 'rating');
         }
 

rating/index.php

@@ -55,7 +55,13 @@ if ($popup) {
     $PAGE->set_pagelayout('popup');
 }
 
-if (!has_capability('moodle/rating:view', $context)) {
+$params = array('contextid' => $contextid,
+                'component' => $component,
+                'ratingarea' => $ratingarea,
+                'itemid' => $itemid,
+                'scaleid' => $scaleid);
+if (!has_capability('moodle/rating:view', $context) ||
+        !component_callback($component, 'rating_can_see_item_ratings', array($params), true)) {
     print_error('noviewrate', 'rating');
 }
 

rating/tests/externallib_test.php

@@ -53,12 +53,15 @@ class core_rating_externallib_testcase extends externallib_advanced_testcase {
         $student = $this->getDataGenerator()->create_user();
         $teacher1 = $this->getDataGenerator()->create_user();
         $teacher2 = $this->getDataGenerator()->create_user();
+        $teacher3 = $this->getDataGenerator()->create_user();
         $studentrole = $DB->get_record('role', array('shortname' => 'student'));
         $teacherrole = $DB->get_record('role', array('shortname' => 'teacher'));
+        unassign_capability('moodle/site:accessallgroups', $teacherrole->id);
 
         $this->getDataGenerator()->enrol_user($student->id,  $course->id, $studentrole->id);
         $this->getDataGenerator()->enrol_user($teacher1->id, $course->id, $teacherrole->id);
         $this->getDataGenerator()->enrol_user($teacher2->id, $course->id, $teacherrole->id);
+        $this->getDataGenerator()->enrol_user($teacher3->id, $course->id, $teacherrole->id);
 
         // Create the forum.
         $record = new stdClass();
@@ -76,13 +79,15 @@ class core_rating_externallib_testcase extends externallib_advanced_testcase {
         $record->userid = $student->id;
         $record->forum = $forum->id;
         $discussion = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record);
+        // Retrieve the first post.
+        $post = $DB->get_record('forum_posts', array('discussion' => $discussion->id));
 
         // Rete the discussion as teacher1.
         $rating1 = new stdClass();
         $rating1->contextid = $contextid;
         $rating1->component = 'mod_forum';
         $rating1->ratingarea = 'post';
-        $rating1->itemid = $discussion->id;
+        $rating1->itemid = $post->id;
         $rating1->rating = 90;
         $rating1->scaleid = 100;
         $rating1->userid = $teacher1->id;
@@ -95,7 +100,7 @@ class core_rating_externallib_testcase extends externallib_advanced_testcase {
         $rating2->contextid = $contextid;
         $rating2->component = 'mod_forum';
         $rating2->ratingarea = 'post';
-        $rating2->itemid = $discussion->id;
+        $rating2->itemid = $post->id;
         $rating2->rating = 95;
         $rating2->scaleid = 100;
         $rating2->userid = $teacher2->id;
@@ -109,7 +114,7 @@ class core_rating_externallib_testcase extends externallib_advanced_testcase {
         // Teachers can see all the ratings.
         $this->setUser($teacher1);
 
-        $ratings = core_rating_external::get_item_ratings('module', $forum->cmid, 'mod_forum', 'post', $discussion->id, 100, '');
+        $ratings = core_rating_external::get_item_ratings('module', $forum->cmid, 'mod_forum', 'post', $post->id, 100, '');
         // We need to execute the return values cleaning process to simulate the web service server.
         $ratings = external_api::clean_returnvalue(core_rating_external::get_item_ratings_returns(), $ratings);
         $this->assertCount(2, $ratings['ratings']);
@@ -127,29 +132,57 @@ class core_rating_externallib_testcase extends externallib_advanced_testcase {
         // Student can see ratings.
         $this->setUser($student);
 
-        $ratings = core_rating_external::get_item_ratings('module', $forum->cmid, 'mod_forum', 'post', $discussion->id, 100, '');
+        $ratings = core_rating_external::get_item_ratings('module', $forum->cmid, 'mod_forum', 'post', $post->id, 100, '');
         // We need to execute the return values cleaning process to simulate the web service server.
         $ratings = external_api::clean_returnvalue(core_rating_external::get_item_ratings_returns(), $ratings);
         $this->assertCount(2, $ratings['ratings']);
 
         // Invalid item.
-        $ratings = core_rating_external::get_item_ratings('module', $forum->cmid, 'mod_forum', 'post', 0, 100, '');
-        // We need to execute the return values cleaning process to simulate the web service server.
-        $ratings = external_api::clean_returnvalue(core_rating_external::get_item_ratings_returns(), $ratings);
-        $this->assertCount(0, $ratings['ratings']);
+        try {
+            $ratings = core_rating_external::get_item_ratings('module', $forum->cmid, 'mod_forum', 'post', 0, 100, '');
+            $this->fail('Exception expected due invalid itemid.');
+        } catch (moodle_exception $e) {
+            $this->assertEquals('invalidrecord', $e->errorcode);
+        }
 
         // Invalid area.
-        $ratings = core_rating_external::get_item_ratings('module', $forum->cmid, 'mod_forum', 'xyz', $discussion->id, 100, '');
-        // We need to execute the return values cleaning process to simulate the web service server.
-        $ratings = external_api::clean_returnvalue(core_rating_external::get_item_ratings_returns(), $ratings);
-        $this->assertCount(0, $ratings['ratings']);
+        try {
+            $ratings = core_rating_external::get_item_ratings('module', $forum->cmid, 'mod_forum', 'xyz', $post->id, 100, '');
+            $this->fail('Exception expected due invalid rating area.');
+        } catch (moodle_exception $e) {
+            $this->assertEquals('invalidratingarea', $e->errorcode);
+        }
 
         // Invalid context. invalid_parameter_exception.
         try {
-            $ratings = core_rating_external::get_item_ratings('module', 0, 'mod_forum', 'post', $discussion->id, 100, '');
+            $ratings = core_rating_external::get_item_ratings('module', 0, 'mod_forum', 'post', $post->id, 100, '');
             $this->fail('Exception expected due invalid context.');
         } catch (invalid_parameter_exception $e) {
             $this->assertEquals('invalidparameter', $e->errorcode);
         }
+
+        // Test for groupmode.
+        set_coursemodule_groupmode($forum->cmid, SEPARATEGROUPS);
+        $group = $this->getDataGenerator()->create_group(array('courseid' => $course->id));
+        groups_add_member($group, $teacher1);
+
+        $discussion->groupid = $group->id;
+        $DB->update_record('forum_discussions', $discussion);
+
+        // Error for teacher3 and 2 ratings for teacher1 should be returned.
+        $this->setUser($teacher1);
+        $ratings = core_rating_external::get_item_ratings('module', $forum->cmid, 'mod_forum', 'post', $post->id, 100, '');
+        // We need to execute the return values cleaning process to simulate the web service server.
+        $ratings = external_api::clean_returnvalue(core_rating_external::get_item_ratings_returns(), $ratings);
+        $this->assertCount(2, $ratings['ratings']);
+
+        $this->setUser($teacher3);
+        try {
+            $ratings = core_rating_external::get_item_ratings('module', $forum->cmid, 'mod_forum', 'post', $post->id, 100, '');
+            $this->fail('Exception expected due invalid group permissions.');
+        } catch (moodle_exception $e) {
+            $this->assertEquals('noviewrate', $e->errorcode);
+        }
+
     }
 }