Skip to content

Commit a3d19ef

Browse files
mdjnelsonJenkins
authored andcommitted
MDL-65365 core_message: prevent users from viewing all conversations
1 parent 888f55e commit a3d19ef

File tree

2 files changed

+32
-2
lines changed

2 files changed

+32
-2
lines changed

message/externallib.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2199,13 +2199,13 @@ public static function get_conversation_messages_parameters() {
21992199
* @param int $limitnum Return a subset comprising this many records in total (optional, required if $limitfrom is set).
22002200
* @param bool $newest True for getting first newest messages, false otherwise.
22012201
* @param int $timefrom The time from the conversation messages to get.
2202-
* @return stdClass The messages and members who have sent some of these messages.
2202+
* @return array The messages and members who have sent some of these messages.
22032203
* @throws moodle_exception
22042204
* @since 3.6
22052205
*/
22062206
public static function get_conversation_messages(int $currentuserid, int $convid, int $limitfrom = 0, int $limitnum = 0,
22072207
bool $newest = false, int $timefrom = 0) {
2208-
global $CFG, $PAGE, $USER;
2208+
global $CFG, $USER;
22092209

22102210
// Check if messaging is enabled.
22112211
if (empty($CFG->messaging)) {
@@ -2229,6 +2229,11 @@ public static function get_conversation_messages(int $currentuserid, int $convid
22292229
throw new moodle_exception('You do not have permission to perform this action.');
22302230
}
22312231

2232+
// Check that the user belongs to the conversation.
2233+
if (!\core_message\api::is_user_in_conversation($params['currentuserid'], $params['convid'])) {
2234+
throw new moodle_exception('User is not part of conversation.');
2235+
}
2236+
22322237
$sort = $newest ? 'timecreated DESC' : 'timecreated ASC';
22332238

22342239
// We need to enforce a one second delay on messages to avoid race conditions of current

message/tests/externallib_test.php

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3945,6 +3945,31 @@ public function test_get_conversation_messages_as_other_user_without_cap() {
39453945
core_message_external::get_conversation_messages($user2->id, $conversation->id);
39463946
}
39473947

3948+
/**
3949+
* Tests get_conversation_messages for retrieving messages as another user not in the conversation.
3950+
*/
3951+
public function test_get_conversation_messages_as_user_not_in_conversation() {
3952+
$this->resetAfterTest(true);
3953+
3954+
// Create some users.
3955+
$user1 = self::getDataGenerator()->create_user();
3956+
$user2 = self::getDataGenerator()->create_user();
3957+
$user3 = self::getDataGenerator()->create_user(); // Not in group.
3958+
3959+
// Create group conversation.
3960+
$conversation = \core_message\api::create_conversation(
3961+
\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP,
3962+
[$user1->id, $user2->id]
3963+
);
3964+
3965+
// The person asking for the messages for a conversation he does not belong to.
3966+
$this->setUser($user3);
3967+
3968+
// Ensure an exception is thrown.
3969+
$this->expectExceptionMessage('User is not part of conversation.');
3970+
core_message_external::get_conversation_messages($user3->id, $conversation->id);
3971+
}
3972+
39483973
/**
39493974
* Tests get_conversation_messages for retrieving messages with messaging disabled.
39503975
*/

0 commit comments

Comments
 (0)