MDL-64525 comments: Should not be able to delete frozen comments
authorNeill Magill <neill.magill@nottingham.ac.uk>
Fri, 9 Aug 2019 14:20:16 +0000 (15:20 +0100)
committerNeill Magill <neill.magill@nottingham.ac.uk>
Mon, 24 Aug 2020 07:00:36 +0000 (08:00 +0100)
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
comment/comment_ajax.php
comment/lib.php
lang/en/error.php

index 41a8b41..1eb3d8e 100644 (file)
@@ -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');
             }
 
index c1fe37c..50caa94 100644 (file)
@@ -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,
index 58f8151..65d81e3 100644 (file)
@@ -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()));
     }
 
     /**
index 3530052..e41176c 100644 (file)
@@ -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';