L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-45303 Web Services: forum_get_discussions ws misses permissions check

Browse Source
This commit is contained in:
Juan Leyva
2014-11-03 21:53:42 +01:00
committed by Eloy Lafuente (stronk7)
parent 3b8876f5ef
commit 3aa9d93c7a
2 changed files with 62 additions and 100 deletions
Download Patch File Download Diff File Expand all files Collapse all files
mod/forum/externallib.php
+48 -46
View File
@@ -196,10 +196,6 @@ class mod_forum_external extends external_api {
// Array to store the forum discussions to return.
$arrdiscussions = array();
// Keep track of the course ids we have performed a require_course_login check on to avoid repeating.
$arrcourseschecked = array();
// Store the modinfo for the forums in an individual courses.
$arrcoursesforuminfo = array();
// Keep track of the users we have looked up in the DB.
$arrusers = array();
@@ -207,52 +203,58 @@ class mod_forum_external extends external_api {
foreach ($forumids as $id) {
// Get the forum object.
$forum = $DB->get_record('forum', array('id' => $id), '*', MUST_EXIST);
// Check that that user can view this course if check not performed yet.
if (!in_array($forum->course, $arrcourseschecked)) {
// Check the user can function in this context.
self::validate_context(context_course::instance($forum->course));
// Add to the array.
$arrcourseschecked[] = $forum->course;
}
// Get the modinfo for the course if we haven't already.
if (!isset($arrcoursesforuminfo[$forum->course])) {
$modinfo = get_fast_modinfo($forum->course);
$arrcoursesforuminfo[$forum->course] = $modinfo->get_instances_of('forum');
}
// Check if this forum does not exist in the modinfo array, should always be false unless DB is borked.
if (empty($arrcoursesforuminfo[$forum->course][$forum->id])) {
throw new moodle_exception('invalidmodule', 'error');
}
// We now have the course module.
$cm = $arrcoursesforuminfo[$forum->course][$forum->id];
// If the forum is not visible throw an exception.
if (!$cm->uservisible) {
throw new moodle_exception('nopermissiontoshow', 'error');
}
$course = get_course($forum->course);
$modinfo = get_fast_modinfo($course);
$forums = $modinfo->get_instances_of('forum');
$cm = $forums[$forum->id];
// Get the module context.
$modcontext = context_module::instance($cm->id);
// Check they have the view forum capability.
// Validate the context.
self::validate_context($modcontext);
require_capability('mod/forum:viewdiscussion', $modcontext);
// Check if they can view full names.
$canviewfullname = has_capability('moodle/site:viewfullnames', $modcontext);
// Get the unreads array, this takes a forum id and returns data for all discussions.
$unreads = array();
if ($cantrack = forum_tp_can_track_forums($forum)) {
if ($forumtracked = forum_tp_is_tracked($forum)) {
$unreads = forum_get_discussions_unread($cm);
// Get the discussions for this forum.
$params = array();
$groupselect = "";
$groupmode = groups_get_activity_groupmode($cm, $course);
if ($groupmode and $groupmode != VISIBLEGROUPS and !has_capability('moodle/site:accessallgroups', $modcontext)) {
// Get all the discussions from all the groups this user belongs to.
$usergroups = groups_get_user_groups($course->id);
if (!empty($usergroups['0'])) {
list($sql, $params) = $DB->get_in_or_equal($usergroups['0']);
$groupselect = "AND (groupid $sql OR groupid = -1)";
}
}
// The forum function returns the replies for all the discussions in a given forum.
$replies = forum_count_discussion_replies($id);
// Get the discussions for this forum.
$order = 'timemodified DESC';
if ($discussions = $DB->get_records('forum_discussions', array('forum' => $id), $order, '*', $limitfrom, $limitnum)) {
foreach ($discussions as $discussion) {
// If the forum is of type qanda and the user has not posted in the discussion
// we need to ensure that they have the required capability.
if ($forum->type == 'qanda' && !forum_user_has_posted($discussion->forum, $discussion->id, $USER->id)) {
require_capability('mod/forum:viewqandawithoutposting', $modcontext);
array_unshift($params, $id);
$select = "forum = ? $groupselect";
if ($discussions = $DB->get_records_select('forum_discussions', $select, $params, 'timemodified DESC', '*',
$limitfrom, $limitnum)) {
// Check if they can view full names.
$canviewfullname = has_capability('moodle/site:viewfullnames', $modcontext);
// Get the unreads array, this takes a forum id and returns data for all discussions.
$unreads = array();
if ($cantrack = forum_tp_can_track_forums($forum)) {
if ($forumtracked = forum_tp_is_tracked($forum)) {
$unreads = forum_get_discussions_unread($cm);
}
}
// The forum function returns the replies for all the discussions in a given forum.
$replies = forum_count_discussion_replies($id);
foreach ($discussions as $discussion) {
// This function checks capabilities, timed discussions, groups and qanda forums posting.
if (!forum_user_can_see_discussion($forum, $discussion, $modcontext)) {
continue;
}
$usernamefields = user_picture::fields();
// If we don't have the users details then perform DB call.
if (empty($arrusers[$discussion->userid])) {
@@ -290,10 +292,10 @@ class mod_forum_external extends external_api {
if (!empty($replies[$discussion->id])) {
$return->numreplies = (int) $replies[$discussion->id]->replies;
$return->lastpost = (int) $replies[$discussion->id]->lastpostid;
} else { // No replies, so the last post will be the first post.
} else { // No replies, so the last post will be the first post.
$return->numreplies = 0;
$return->lastpost = (int) $discussion->firstpost;
}
}
// Get the last post as well as the user who made it.
$lastpost = $DB->get_record('forum_posts', array('id' => $return->lastpost), '*', MUST_EXIST);
if (empty($arrusers[$lastpost->userid])) {
mod/forum/tests/externallib_test.php
+14 -54
View File
@@ -177,11 +177,6 @@ class mod_forum_external_testcase extends externallib_advanced_testcase {
$record->trackingtype = FORUM_TRACKING_FORCED;
$forum2 = self::getDataGenerator()->create_module('forum', $record);
// Third forum where we will only have one discussion with no replies.
$record = new stdClass();
$record->course = $course2->id;
$forum3 = self::getDataGenerator()->create_module('forum', $record);
// Add discussions to the forums.
$record = new stdClass();
$record->course = $course1->id;
@@ -195,12 +190,6 @@ class mod_forum_external_testcase extends externallib_advanced_testcase {
$record->forum = $forum2->id;
$discussion2 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record);
$record = new stdClass();
$record->course = $course2->id;
$record->userid = $user2->id;
$record->forum = $forum3->id;
$discussion3 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record);
// Add three replies to the discussion 1 from different users.
$record = new stdClass();
$record->discussion = $discussion1->id;
@@ -227,12 +216,12 @@ class mod_forum_external_testcase extends externallib_advanced_testcase {
$discussion2reply2 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_post($record);
// Check the forums were correctly created.
$this->assertEquals(3, $DB->count_records_select('forum', 'id = :forum1 OR id = :forum2 OR id = :forum3',
array('forum1' => $forum1->id, 'forum2' => $forum2->id, 'forum3' => $forum3->id)));
$this->assertEquals(2, $DB->count_records_select('forum', 'id = :forum1 OR id = :forum2',
array('forum1' => $forum1->id, 'forum2' => $forum2->id)));
// Check the discussions were correctly created.
$this->assertEquals(3, $DB->count_records_select('forum_discussions', 'forum = :forum1 OR forum = :forum2
OR id = :forum3', array('forum1' => $forum1->id, 'forum2' => $forum2->id, 'forum3' => $forum3->id)));
$this->assertEquals(2, $DB->count_records_select('forum_discussions', 'forum = :forum1 OR forum = :forum2',
array('forum1' => $forum1->id, 'forum2' => $forum2->id)));
// Check the posts were correctly created, don't forget each discussion created also creates a post.
$this->assertEquals(7, $DB->count_records_select('forum_posts', 'discussion = :discussion1 OR discussion = :discussion2',
@@ -261,11 +250,6 @@ class mod_forum_external_testcase extends externallib_advanced_testcase {
$newrole = create_role('Role 2', 'role2', 'Role 2 description');
$this->assignUserCapability('mod/forum:viewdiscussion', $context->id, $newrole);
// Assign capabilities to view discussions for forum 3.
$cm = get_coursemodule_from_id('forum', $forum3->cmid, 0, false, MUST_EXIST);
$context = context_module::instance($cm->id);
$this->assignUserCapability('mod/forum:viewdiscussion', $context->id, $newrole);
// Create what we expect to be returned when querying the forums.
$expecteddiscussions = array();
$expecteddiscussions[$discussion1->id] = array(
@@ -322,47 +306,21 @@ class mod_forum_external_testcase extends externallib_advanced_testcase {
'lastuserpicture' => $user3->picture,
'lastuseremail' => $user3->email
);
$expecteddiscussions[$discussion3->id] = array(
'id' => $discussion3->id,
'course' => $discussion3->course,
'forum' => $discussion3->forum,
'name' => $discussion3->name,
'firstpost' => $discussion3->firstpost,
'userid' => $discussion3->userid,
'groupid' => $discussion3->groupid,
'assessed' => $discussion3->assessed,
'timemodified' => $discussion3->timemodified,
'usermodified' => $discussion3->usermodified,
'timestart' => $discussion3->timestart,
'timeend' => $discussion3->timeend,
'firstuserfullname' => fullname($user2),
'firstuserimagealt' => $user2->imagealt,
'firstuserpicture' => $user2->picture,
'firstuseremail' => $user2->email,
'subject' => $discussion3->name,
'numreplies' => 0,
'numunread' => 1,
'lastpost' => $discussion3->firstpost,
'lastuserid' => $user2->id,
'lastuserfullname' => fullname($user2),
'lastuserimagealt' => $user2->imagealt,
'lastuserpicture' => $user2->picture,
'lastuseremail' => $user2->email
);
// Call the external function passing forum ids.
$discussions = mod_forum_external::get_forum_discussions(array($forum1->id, $forum2->id, $forum3->id));
$discussions = mod_forum_external::get_forum_discussions(array($forum1->id, $forum2->id));
external_api::clean_returnvalue(mod_forum_external::get_forum_discussions_returns(), $discussions);
$this->assertEquals($expecteddiscussions, $discussions);
// Some debugging is going to be produced, this is because we switch PAGE contexts in the get_forum_discussions function,
// the switch happens when the validate_context function is called inside a foreach loop.
// See MDL-41746 for more information.
$this->assertDebuggingCalled();
// Remove the users post from the qanda forum and ensure they can not return the discussion.
$DB->delete_records('forum_posts', array('id' => $discussion2reply1->id));
try {
mod_forum_external::get_forum_discussions(array($forum2->id));
$this->fail('Exception expected due to attempting to access qanda forum without posting.');
} catch (moodle_exception $e) {
$this->assertEquals('nopermissions', $e->errorcode);
}
$discussions = mod_forum_external::get_forum_discussions(array($forum2->id));
$discussions = external_api::clean_returnvalue(mod_forum_external::get_forum_discussions_returns(), $discussions);
$this->assertEquals(0, count($discussions));
// Call without required view discussion capability.
$this->unassignUserCapability('mod/forum:viewdiscussion', null, null, $course1->id);
@@ -372,6 +330,7 @@ class mod_forum_external_testcase extends externallib_advanced_testcase {
} catch (moodle_exception $e) {
$this->assertEquals('nopermissions', $e->errorcode);
}
$this->assertDebuggingCalled();
// Unenrol user from second course.
$enrol->unenrol_user($instance2, $user1->id);
@@ -383,6 +342,7 @@ class mod_forum_external_testcase extends externallib_advanced_testcase {
} catch (moodle_exception $e) {
$this->assertEquals('requireloginerror', $e->errorcode);
}
$this->assertDebuggingCalled();
}
/**