Skip to content

Commit

Permalink
MDL-68900 mod_forum: permission check for user who is viewing timed post
Browse files Browse the repository at this point in the history
Pass current user object to post builder as argument, so that the permission to view timed post
will check with current user, who is viewing the posts instead of user who made that post.
  • Loading branch information
sumitnegi933 committed Oct 8, 2020
1 parent e049d30 commit bd6449f
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 27 deletions.
2 changes: 1 addition & 1 deletion mod/forum/classes/local/builders/exported_posts.php
Expand Up @@ -110,7 +110,7 @@ public function __construct(
* Note: Some posts will be removed as part of the build process according to capabilities.
* A one-to-one mapping should not be expected.
*
* @param stdClass $user The user to export the posts for.
* @param stdClass $user The user who is viewing the posts.
* @param forum_entity[] $forums A list of all forums that each of the $discussions belong to
* @param discussion_entity[] $discussions A list of all discussions that each of the $posts belong to
* @param post_entity[] $posts The list of posts to export.
Expand Down
4 changes: 2 additions & 2 deletions mod/forum/externallib.php
Expand Up @@ -2245,7 +2245,7 @@ public static function get_discussion_posts_by_userid(int $userid = 0, int $cmid
$parentposts = [];
if ($parentids) {
$parentposts = $postbuilder->build(
$user,
$USER,
[$forum],
[$discussion],
$postvault->get_from_ids(array_values($parentids))
Expand All @@ -2261,7 +2261,7 @@ public static function get_discussion_posts_by_userid(int $userid = 0, int $cmid
'timecreated' => $firstpost->get_time_created(),
'authorfullname' => $discussionauthor->get_full_name(),
'posts' => [
'userposts' => $postbuilder->build($user, [$forum], [$discussion], $posts),
'userposts' => $postbuilder->build($USER, [$forum], [$discussion], $posts),
'parentposts' => $parentposts,
],
];
Expand Down
72 changes: 48 additions & 24 deletions mod/forum/tests/externallib_test.php
Expand Up @@ -2611,6 +2611,7 @@ public function test_delete_post_other_user_post() {
* Test get forum posts by user id.
*/
public function test_mod_forum_get_discussion_posts_by_userid() {
global $DB;
$this->resetAfterTest(true);

$urlfactory = mod_forum\local\container::get_url_factory();
Expand Down Expand Up @@ -2722,9 +2723,20 @@ public function test_mod_forum_get_discussion_posts_by_userid() {

// Following line enrol and assign default role id to the user.
// So the user automatically gets mod/forum:viewdiscussion on all forums of the course.
$this->getDataGenerator()->enrol_user($user1->id, $course1->id);
$this->getDataGenerator()->enrol_user($user1->id, $course1->id, 'teacher');
$this->getDataGenerator()->enrol_user($user2->id, $course1->id);

// Changed display period for the discussions in past.
$time = time();
$discussion = new \stdClass();
$discussion->id = $discussion1->id;
$discussion->timestart = $time - 200;
$discussion->timeend = $time - 100;
$DB->update_record('forum_discussions', $discussion);
$discussion = new \stdClass();
$discussion->id = $discussion2->id;
$discussion->timestart = $time - 200;
$discussion->timeend = $time - 100;
$DB->update_record('forum_discussions', $discussion);
// Create what we expect to be returned when querying the discussion.
$expectedposts = array(
'discussions' => array(),
Expand Down Expand Up @@ -2773,34 +2785,36 @@ public function test_mod_forum_get_discussion_posts_by_userid() {
'view' => true,
'edit' => true,
'delete' => true,
'split' => false,
'split' => true,
'reply' => true,
'export' => false,
'controlreadstatus' => false,
'canreplyprivately' => false,
'canreplyprivately' => true,
'selfenrol' => false
],
'urls' => [
'view' => $urlfactory->get_view_post_url_from_post_id(
$discussion1reply1->discussion, $discussion1reply1->id)->out(false),
$discussion1reply1->discussion, $discussion1reply1->id)->out(false),
'viewisolated' => $isolatedurluser->out(false),
'viewparent' => $urlfactory->get_view_post_url_from_post_id(
$discussion1reply1->discussion, $discussion1reply1->parent)->out(false),
$discussion1reply1->discussion, $discussion1reply1->parent)->out(false),
'edit' => (new moodle_url('/mod/forum/post.php', [
'edit' => $discussion1reply1->id
'edit' => $discussion1reply1->id
]))->out(false),
'delete' => (new moodle_url('/mod/forum/post.php', [
'delete' => $discussion1reply1->id
'delete' => $discussion1reply1->id
]))->out(false),
'split' => (new moodle_url('/mod/forum/post.php', [
'prune' => $discussion1reply1->id
]))->out(false),
'split' => null,
'reply' => (new moodle_url('/mod/forum/post.php#mformforum', [
'reply' => $discussion1reply1->id
'reply' => $discussion1reply1->id
]))->out(false),
'export' => null,
'markasread' => null,
'markasunread' => null,
'discuss' => $urlfactory->get_discussion_view_url_from_discussion_id(
$discussion1reply1->discussion)->out(false),
$discussion1reply1->discussion)->out(false),
],
]
],
Expand Down Expand Up @@ -2833,22 +2847,26 @@ public function test_mod_forum_get_discussion_posts_by_userid() {
'charcount' => null,
'capabilities' => [
'view' => true,
'edit' => false,
'delete' => false,
'edit' => true,
'delete' => true,
'split' => false,
'reply' => true,
'export' => false,
'controlreadstatus' => false,
'canreplyprivately' => false,
'canreplyprivately' => true,
'selfenrol' => false
],
'urls' => [
'view' => $urlfactory->get_view_post_url_from_post_id(
$discussion1firstpostobject->discussion, $discussion1firstpostobject->id)->out(false),
'viewisolated' => $isolatedurlparent->out(false),
'viewparent' => null,
'edit' => null,
'delete' => null,
'edit' => (new moodle_url('/mod/forum/post.php', [
'edit' => $discussion1firstpostobject->id
]))->out(false),
'delete' => (new moodle_url('/mod/forum/post.php', [
'delete' => $discussion1firstpostobject->id
]))->out(false),
'split' => null,
'reply' => (new moodle_url('/mod/forum/post.php#mformforum', [
'reply' => $discussion1firstpostobject->id
Expand Down Expand Up @@ -2906,11 +2924,11 @@ public function test_mod_forum_get_discussion_posts_by_userid() {
'view' => true,
'edit' => true,
'delete' => true,
'split' => false,
'split' => true,
'reply' => true,
'export' => false,
'controlreadstatus' => false,
'canreplyprivately' => false,
'canreplyprivately' => true,
'selfenrol' => false
],
'urls' => [
Expand All @@ -2925,7 +2943,9 @@ public function test_mod_forum_get_discussion_posts_by_userid() {
'delete' => (new moodle_url('/mod/forum/post.php', [
'delete' => $discussion2reply1->id
]))->out(false),
'split' => null,
'split' => (new moodle_url('/mod/forum/post.php', [
'prune' => $discussion2reply1->id
]))->out(false),
'reply' => (new moodle_url('/mod/forum/post.php#mformforum', [
'reply' => $discussion2reply1->id
]))->out(false),
Expand Down Expand Up @@ -2966,22 +2986,26 @@ public function test_mod_forum_get_discussion_posts_by_userid() {
'charcount' => null,
'capabilities' => [
'view' => true,
'edit' => false,
'delete' => false,
'edit' => true,
'delete' => true,
'split' => false,
'reply' => true,
'export' => false,
'controlreadstatus' => false,
'canreplyprivately' => false,
'canreplyprivately' => true,
'selfenrol' => false
],
'urls' => [
'view' => $urlfactory->get_view_post_url_from_post_id(
$discussion2firstpostobject->discussion, $discussion2firstpostobject->id)->out(false),
'viewisolated' => $isolatedurlparent->out(false),
'viewparent' => null,
'edit' => null,
'delete' => null,
'edit' => (new moodle_url('/mod/forum/post.php', [
'edit' => $discussion2firstpostobject->id
]))->out(false),
'delete' => (new moodle_url('/mod/forum/post.php', [
'delete' => $discussion2firstpostobject->id
]))->out(false),
'split' => null,
'reply' => (new moodle_url('/mod/forum/post.php#mformforum', [
'reply' => $discussion2firstpostobject->id
Expand Down
3 changes: 3 additions & 0 deletions mod/forum/upgrade.txt
Expand Up @@ -9,6 +9,9 @@ information provided here is intended especially for developers.
for each post in the database when posts are created or updated. For posts that existed prior to a Moodle 3.8 upgrade, these
are calculated by the refresh_forum_post_counts ad-hoc task in chunks of 5000 posts by default. Site admins are able to modify this
default, by setting $CFG->forumpostcountchunksize to the required integer value.
* Changes in external function mod_forum_external::get_discussion_posts_by_userid
$postbuilder build the posts of given user in respect of the permission check of current user $USER who is viewing the user posts,
it could be teacher or privileged user who can view student post or it can be the user himself.

=== 3.7 ===
* Changed the forum discussion rendering to use templates rather than print functions.
Expand Down

0 comments on commit bd6449f

Please sign in to comment.