From 2987182e16dcd6b2df0f03d70c8b21e2739e4324 Mon Sep 17 00:00:00 2001 From: Neill Magill Date: Fri, 9 Aug 2019 15:20:16 +0100 Subject: [PATCH] MDL-64525 comments: Should not be able to delete frozen comments Before this change in a frozen context a user would be able to delete their own comments. After this change a user will only be able to delete their own comments if they still have the capability to post new comments or the capability to delete comments from any user. --- comment/classes/external.php | 2 +- comment/comment_ajax.php | 4 +-- comment/lib.php | 51 +++++++++++++++++++++++++++--------- lang/en/error.php | 2 +- 4 files changed, 43 insertions(+), 16 deletions(-) diff --git a/comment/classes/external.php b/comment/classes/external.php index 41a8b4108bc..1eb3d8e07e1 100644 --- a/comment/classes/external.php +++ b/comment/classes/external.php @@ -337,7 +337,7 @@ class core_comment_external extends external_api { $args->area = $commentrecord->commentarea; $manager = new comment($args); - if ($commentrecord->userid != $USER->id && !$manager->can_delete($commentrecord->id)) { + if (!$manager->can_delete($commentrecord)) { throw new comment_exception('nopermissiontodelentry'); } diff --git a/comment/comment_ajax.php b/comment/comment_ajax.php index c1fe37c00bd..50caa94be42 100644 --- a/comment/comment_ajax.php +++ b/comment/comment_ajax.php @@ -91,8 +91,8 @@ switch ($action) { } break; case 'delete': - $comment_record = $DB->get_record('comments', array('id'=>$commentid)); - if ($manager->can_delete($commentid) || $comment_record->userid == $USER->id) { + $comment = $DB->get_record('comments', ['id' => $commentid]); + if ($manager->can_delete($comment)) { if ($manager->delete($commentid)) { $result = array( 'client_id' => $client_id, diff --git a/comment/lib.php b/comment/lib.php index 58f8151d380..65d81e30cbd 100644 --- a/comment/lib.php +++ b/comment/lib.php @@ -589,8 +589,7 @@ class comment { $c->avatar = $OUTPUT->user_picture($u, array('size'=>18)); $c->userid = $u->id; - $candelete = $this->can_delete($c->id); - if (($USER->id == $u->id) || !empty($candelete)) { + if ($this->can_delete($c)) { $c->delete = true; } $comments[] = $c; @@ -800,16 +799,22 @@ class comment { /** * Delete a comment * - * @param int $commentid + * @param int|stdClass $comment The id of a comment, or a comment record. * @return bool */ - public function delete($commentid) { - global $DB, $USER; - $candelete = has_capability('moodle/comment:delete', $this->context); - if (!$comment = $DB->get_record('comments', array('id'=>$commentid))) { + public function delete($comment) { + global $DB; + if (is_object($comment)) { + $commentid = $comment->id; + } else { + $commentid = $comment; + $comment = $DB->get_record('comments', ['id' => $commentid]); + } + + if (!$comment) { throw new comment_exception('dbupdatefailed'); } - if (!($USER->id == $comment->userid || !empty($candelete))) { + if (!$this->can_delete($comment)) { throw new comment_exception('nopermissiontocomment'); } $DB->delete_records('comments', array('id'=>$commentid)); @@ -976,13 +981,35 @@ class comment { } /** - * Returns true if the user can delete this comment - * @param int $commentid + * Returns true if the user can delete this comment. + * + * The user can delete comments if it is one they posted and they can still make posts, + * or they have the capability to delete comments. + * + * A database call is avoided if a comment record is passed. + * + * @param int|stdClass $comment The id of a comment, or a comment record. * @return bool */ - public function can_delete($commentid) { + public function can_delete($comment) { + global $USER, $DB; + if (is_object($comment)) { + $commentid = $comment->id; + } else { + $commentid = $comment; + } + $this->validate(array('commentid'=>$commentid)); - return has_capability('moodle/comment:delete', $this->context); + + if (!is_object($comment)) { + // Get the comment record from the database. + $comment = $DB->get_record('comments', array('id' => $commentid), 'id, userid', MUST_EXIST); + } + + $hascapability = has_capability('moodle/comment:delete', $this->context); + $owncomment = $USER->id == $comment->userid; + + return ($hascapability || ($owncomment && $this->can_post())); } /** diff --git a/lang/en/error.php b/lang/en/error.php index 6a728796b47..828a2f52cc5 100644 --- a/lang/en/error.php +++ b/lang/en/error.php @@ -440,7 +440,7 @@ $string['noparticipants'] = 'No participants found for this course'; $string['noparticipatorycms'] = 'Sorry, but you have no participatory course modules to report on'; $string['nopermissions'] = 'Sorry, but you do not currently have permissions to do that ({$a}).'; $string['nopermissiontocomment'] = 'You can\'t add comments'; -$string['nopermissiontodelentry'] = 'You can\'t delete other people\'s entries!'; +$string['nopermissiontodelentry'] = 'You can\'t delete this comment!'; $string['nopermissiontoeditcomment'] = 'You can\'t edit other people\'s comments!'; $string['nopermissiontohide'] = 'No permission to hide!'; $string['nopermissiontoimportact'] = 'You do not have the required permissions to import activities to this course';