L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-61742 messaging: Do not send messages from inactive providers

Browse Source 
Before this patch, we only checked that the given provider has been
configured in the user or system preferences. However, if the provider's
component is disabled, it does not even appear in these preferences.
Additionally, there was no check that the message / notification
provider is among providers allowed to be consumed by the recipient.

The patch checks that the message origin is among providers returned by
the message_get_providers_for_user() so disabled plugins can't act as
sources of messages and users can't receive messages from providers they
do not have capability for. This mitigates the risk of abusing a plugin
as a source of spam, for example.

Unit test is fixed and extended. When the $CFG->messaging is disabled,
no messages between users should be sent (I can't understand why the
unit test was written in an opposite way). Added assertions for the
raised debugging message.
This commit is contained in:
David Mudrák
2018-08-31 12:15:30 +02:00
parent 175b3708c9
commit 39d2c68856
4 changed files with 53 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lib/messagelib.php
+16 -1
View File
@@ -63,7 +63,8 @@ function message_send(\core\message\message $eventdata) {
// Fetch default (site) preferences
$defaultpreferences = get_message_output_default_preferences();
$preferencebase = $eventdata->component.'_'.$eventdata->name;
// If message provider is disabled then don't do any processing.
// If the message provider is disabled via preferences, then don't send the message.
if (!empty($defaultpreferences->{$preferencebase.'_disable'})) {
return $messageid;
}
@@ -88,6 +89,20 @@ function message_send(\core\message\message $eventdata) {
return false;
}
// If the provider's component is disabled or the user can't receive messages from it, don't send the message.
$isproviderallowed = false;
foreach (message_get_providers_for_user($eventdata->userto->id) as $provider) {
if ($provider->component === $eventdata->component && $provider->name === $eventdata->name) {
$isproviderallowed = true;
break;
}
}
if (!$isproviderallowed) {
debugging('Attempt to send msg from a provider '.$eventdata->component.'/'.$eventdata->name.
' that is inactive or not allowed for the user id='.$eventdata->userto->id, DEBUG_NORMAL);
return false;
}
// Verify all necessary data fields are present.
if (!isset($eventdata->userto->auth) or !isset($eventdata->userto->suspended)
or !isset($eventdata->userto->deleted) or !isset($eventdata->userto->emailstop)) {
lib/phpunit/tests/advanced_test.php
+7 -12
View File
@@ -516,21 +516,16 @@ class core_phpunit_advanced_testcase extends advanced_testcase {
$message3->smallmessage = 'small message';
$message3->notification = 0;
try {
message_send($message3);
$this->fail('coding expcetion expected if invalid component specified');
} catch (moodle_exception $e) {
$this->assertInstanceOf('coding_exception', $e);
}
$this->assertFalse(message_send($message3));
$this->assertDebuggingCalled('Attempt to send msg from a provider xxxx_yyyyy/instantmessage '.
'that is inactive or not allowed for the user id='.$user1->id);
$message3->component = 'moodle';
$message3->name = 'yyyyyy';
try {
message_send($message3);
$this->fail('coding expcetion expected if invalid name specified');
} catch (moodle_exception $e) {
$this->assertInstanceOf('coding_exception', $e);
}
$this->assertFalse(message_send($message3));
$this->assertDebuggingCalled('Attempt to send msg from a provider moodle/yyyyyy '.
'that is inactive or not allowed for the user id='.$user1->id);
message_send($message1);
$this->assertEquals(1, $sink->count());
lib/tests/messagelib_test.php
+10 -6
View File
@@ -287,6 +287,8 @@ class core_messagelib_testcase extends advanced_testcase {
$this->assertInstanceOf('coding_exception', $e);
}
$this->assertCount(0, $sink->get_messages());
$this->assertDebuggingCalled('Attempt to send msg from a provider xxxxx/instantmessage '.
'that is inactive or not allowed for the user id='.$user2->id);
$message->component = 'moodle';
$message->name = 'xxx';
@@ -297,6 +299,8 @@ class core_messagelib_testcase extends advanced_testcase {
$this->assertInstanceOf('coding_exception', $e);
}
$this->assertCount(0, $sink->get_messages());
$this->assertDebuggingCalled('Attempt to send msg from a provider moodle/xxx '.
'that is inactive or not allowed for the user id='.$user2->id);
$sink->close();
$this->assertFalse($DB->record_exists('messages', array()));
@@ -430,6 +434,7 @@ class core_messagelib_testcase extends advanced_testcase {
$this->assertInstanceOf('\core\event\message_sent', $events[0]);
$eventsink->clear();
// No messages are sent when the feature is disabled.
$CFG->messaging = 0;
$message = new \core\message\message();
@@ -446,20 +451,19 @@ class core_messagelib_testcase extends advanced_testcase {
$message->notification = '0';
$messageid = message_send($message);
$this->assertFalse($messageid);
$this->assertDebuggingCalled('Attempt to send msg from a provider moodle/instantmessage '.
'that is inactive or not allowed for the user id='.$user2->id);
$emails = $sink->get_messages();
$this->assertCount(0, $emails);
$savedmessage = $DB->get_record('messages', array('id' => $messageid), '*', MUST_EXIST);
$sink->clear();
$this->assertTrue($DB->record_exists('message_user_actions', array('userid' => $user2->id, 'messageid' => $messageid,
'action' => \core_message\api::MESSAGE_ACTION_READ)));
$DB->delete_records('messages', array());
$DB->delete_records('message_user_actions', array());
$events = $eventsink->get_events();
$this->assertCount(2, $events);
$this->assertInstanceOf('\core\event\message_sent', $events[0]);
$this->assertInstanceOf('\core\event\message_viewed', $events[1]);
$this->assertCount(0, $events);
$eventsink->clear();
// Example of a message that is sent and viewed.
$CFG->messaging = 1;
$message = new \core\message\message();
message/tests/externallib_test.php
+20 -1
View File
@@ -534,9 +534,10 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
// Now, create some notifications...
// We are creating fake notifications but based on real ones.
// This one omits notification = 1.
// This one comes from a disabled plugin's provider and therefore is not sent.
$eventdata = new \core\message\message();
$eventdata->courseid = $course->id;
$eventdata->notification = 1;
$eventdata->modulename = 'moodle';
$eventdata->component = 'enrol_paypal';
$eventdata->name = 'paypal_enrolment';
@@ -548,6 +549,24 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
$eventdata->fullmessagehtml = '';
$eventdata->smallmessage = '';
message_send($eventdata);
$this->assertDebuggingCalled('Attempt to send msg from a provider enrol_paypal/paypal_enrolment '.
'that is inactive or not allowed for the user id='.$user1->id);
// This one omits notification = 1.
$message = new \core\message\message();
$message->courseid = $course->id;
$message->component = 'enrol_manual';
$message->name = 'expiry_notification';
$message->userfrom = $user2;
$message->userto = $user1;
$message->subject = 'Test: This is not a notification but otherwise is valid';
$message->fullmessage = 'Test: Full message';
$message->fullmessageformat = FORMAT_MARKDOWN;
$message->fullmessagehtml = markdown_to_html($message->fullmessage);
$message->smallmessage = $message->subject;
$message->contexturlname = $course->fullname;
$message->contexturl = (string)new moodle_url('/course/view.php', array('id' => $course->id));
message_send($message);
$message = new \core\message\message();
$message->courseid = $course->id;