MDL-65365 core_message: prevent users from viewing all conversations

mgleeson · May 8, 2019 · 2904a7f · 2904a7f
1 parent 2018f4e
commit 2904a7f
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 2 deletions.
diff --git a/message/externallib.php b/message/externallib.php
@@ -2446,13 +2446,13 @@ public static function get_conversation_messages_parameters() {
      * @param  int $limitnum Return a subset comprising this many records in total (optional, required if $limitfrom is set).
      * @param  bool $newest True for getting first newest messages, false otherwise.
      * @param  int  $timefrom The time from the conversation messages to get.
-     * @return stdClass The messages and members who have sent some of these messages.
+     * @return array The messages and members who have sent some of these messages.
      * @throws moodle_exception
      * @since 3.6
      */
     public static function get_conversation_messages(int $currentuserid, int $convid, int $limitfrom = 0, int $limitnum = 0,
                                                          bool $newest = false, int $timefrom = 0) {
-        global $CFG, $PAGE, $USER;
+        global $CFG, $USER;
 
         // Check if messaging is enabled.
         if (empty($CFG->messaging)) {
@@ -2476,6 +2476,11 @@ public static function get_conversation_messages(int $currentuserid, int $convid
             throw new moodle_exception('You do not have permission to perform this action.');
         }
 
+        // Check that the user belongs to the conversation.
+        if (!\core_message\api::is_user_in_conversation($params['currentuserid'], $params['convid'])) {
+            throw new moodle_exception('User is not part of conversation.');
+        }
+
         $sort = $newest ? 'timecreated DESC' : 'timecreated ASC';
 
         // We need to enforce a one second delay on messages to avoid race conditions of current

diff --git a/message/tests/externallib_test.php b/message/tests/externallib_test.php
@@ -4148,6 +4148,31 @@ public function test_get_conversation_messages_as_other_user_without_cap() {
         core_message_external::get_conversation_messages($user2->id, $conversation->id);
     }
 
+    /**
+     * Tests get_conversation_messages for retrieving messages as another user not in the conversation.
+     */
+    public function test_get_conversation_messages_as_user_not_in_conversation() {
+        $this->resetAfterTest(true);
+
+        // Create some users.
+        $user1 = self::getDataGenerator()->create_user();
+        $user2 = self::getDataGenerator()->create_user();
+        $user3 = self::getDataGenerator()->create_user(); // Not in group.
+
+        // Create group conversation.
+        $conversation = \core_message\api::create_conversation(
+            \core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP,
+            [$user1->id, $user2->id]
+        );
+
+        // The person asking for the messages for a conversation he does not belong to.
+        $this->setUser($user3);
+
+        // Ensure an exception is thrown.
+        $this->expectExceptionMessage('User is not part of conversation.');
+        core_message_external::get_conversation_messages($user3->id, $conversation->id);
+    }
+
     /**
      * Tests get_conversation_messages for retrieving messages with messaging disabled.
      */