MDL-71239 calendar: disallow manager role users edit user events

calendar/externallib.php

@@ -815,10 +815,10 @@ class core_calendar_external extends external_api {
         $warnings = array();
 
         $eventvault = event_container::get_event_vault();
-        if ($event = $eventvault->get_event_by_id($eventid)) {
+        if ($event = $eventvault->get_event_by_id($params['eventid'])) {
             $mapper = event_container::get_event_mapper();
             if (!calendar_view_event_allowed($mapper->from_event_to_legacy_event($event))) {
-                $event = null;
+                throw new moodle_exception('nopermissiontoviewcalendar', 'error');
             }
         }
 
@@ -826,7 +826,7 @@ class core_calendar_external extends external_api {
             // We can't return a warning in this case because the event is not optional.
             // We don't know the context for the event and it's not worth loading it.
             $syscontext = context_system::instance();
-            throw new \required_capability_exception($syscontext, 'moodle/course:view', 'nopermission', '');
+            throw new \required_capability_exception($syscontext, 'moodle/course:view', 'nopermissions', 'error');
         }
 
         $cache = new events_related_objects_cache([$event]);

calendar/lib.php

@@ -2181,6 +2181,24 @@ function calendar_set_filters(array $courseeventsfrom, $ignorefilters = false, s
     return array($courses, $group, $userid);
 }
 
+/**
+ * Can current user manage a non user event in system context.
+ *
+ * @param calendar_event|stdClass $event event object
+ * @return boolean
+ */
+function calendar_can_manage_non_user_event_in_system($event) {
+    $sitecontext = \context_system::instance();
+    $isuserevent = $event->eventtype == 'user';
+    $canmanageentries = has_capability('moodle/calendar:manageentries', $sitecontext);
+    // If user has manageentries at site level and it's not user event, return true.
+    if ($canmanageentries && !$isuserevent) {
+        return true;
+    }
+
+    return false;
+}
+
 /**
  * Return the capability for viewing a calendar event.
  *
@@ -2195,10 +2213,7 @@ function calendar_view_event_allowed(calendar_event $event) {
         return true;
     }
 
-    // If a user can manage events at the site level they can see any event.
-    $sitecontext = \context_system::instance();
-    // If user has manageentries at site level, return true.
-    if (has_capability('moodle/calendar:manageentries', $sitecontext)) {
+    if (calendar_can_manage_non_user_event_in_system($event)) {
         return true;
     }
 
@@ -2254,11 +2269,7 @@ function calendar_view_event_allowed(calendar_event $event) {
 
         return can_access_course(get_course($event->courseid));
     } else if ($event->userid) {
-        if ($event->userid != $USER->id) {
-            // No-one can ever see another users events.
-            return false;
-        }
-        return true;
+        return calendar_can_manage_user_event($event);
     } else {
         throw new moodle_exception('unknown event type');
     }
@@ -2331,10 +2342,7 @@ function calendar_edit_event_allowed($event, $manualedit = false) {
         }
     }
 
-    $sitecontext = \context_system::instance();
-
-    // If user has manageentries at site level, return true.
-    if (has_capability('moodle/calendar:manageentries', $sitecontext)) {
+    if (calendar_can_manage_non_user_event_in_system($event)) {
         return true;
     }
 
@@ -2358,7 +2366,38 @@ function calendar_edit_event_allowed($event, $manualedit = false) {
         // If course is not set, but userid id set, it's a user event.
         return (has_capability('moodle/calendar:manageownentries', $event->context));
     } else if (!empty($event->userid)) {
-        return (has_capability('moodle/calendar:manageentries', $event->context));
+        return calendar_can_manage_user_event($event);
+    }
+
+    return false;
+}
+
+/**
+ * Can current user edit/delete/add an user event?
+ *
+ * @param calendar_event|stdClass $event event object
+ * @return bool
+ */
+function calendar_can_manage_user_event($event): bool {
+    global $USER;
+
+    if (!($event instanceof \calendar_event)) {
+        $event = new \calendar_event(clone($event));
+    }
+
+    $canmanage = has_capability('moodle/calendar:manageentries', $event->context);
+    $canmanageown = has_capability('moodle/calendar:manageownentries', $event->context);
+    $ismyevent = $event->userid == $USER->id;
+    $isadminevent = is_siteadmin($event->userid);
+
+    if ($canmanageown && $ismyevent) {
+        return true;
+    }
+
+    // In site context, user must have login and calendar:manageentries permissions
+    // ... to manage other user's events except admin users.
+    if ($canmanage && !$isadminevent) {
+        return true;
     }
 
     return false;
@@ -2670,10 +2709,7 @@ function calendar_add_event_allowed($event) {
         return false;
     }
 
-    $sitecontext = \context_system::instance();
-
-    // If user has manageentries at site level, always return true.
-    if (has_capability('moodle/calendar:manageentries', $sitecontext)) {
+    if (calendar_can_manage_non_user_event_in_system($event)) {
         return true;
     }
 
@@ -2692,10 +2728,7 @@ function calendar_add_event_allowed($event) {
                     (has_capability('moodle/calendar:managegroupentries', $event->context)
                         && groups_is_member($event->groupid)));
         case 'user':
-            if ($event->userid == $USER->id) {
-                return (has_capability('moodle/calendar:manageownentries', $event->context));
-            }
-        // There is intentionally no 'break'.
+            return calendar_can_manage_user_event($event);
         case 'site':
             return has_capability('moodle/calendar:manageentries', $event->context);
         default:

calendar/tests/events_test.php

@@ -223,7 +223,8 @@ class core_calendar_events_testcase extends advanced_testcase {
      * Tests for calendar_event_updated event.
      */
     public function test_calendar_event_updated_toggle_visibility() {
-        global $DB, $SITE;
+        global $DB;
+        $siteid = 0;
 
         $this->resetAfterTest();
 
@@ -242,10 +243,9 @@ class core_calendar_events_testcase extends advanced_testcase {
         $event = $events[0];
         $this->assertInstanceOf('\core\event\calendar_event_updated', $event);
         $this->assertEquals('event', $event->objecttable);
-        $this->assertEquals($SITE->id, $event->courseid);
+        $this->assertEquals($siteid, $event->courseid);
         $this->assertEquals($calevent->context, $event->get_context());
-        $expectedlog = array($SITE->id, 'calendar', 'edit', 'event.php?action=edit&id=' . $calevent->id ,
-            $calevent->name);
+        $expectedlog = [$siteid, 'calendar', 'edit', 'event.php?action=edit&id=' . $calevent->id , $calevent->name];
         $this->assertEventLegacyLogData($expectedlog, $event);
         $other = array('repeatid' => 0, 'timestart' => $time, 'name' => 'Some wickedly awesome event yo!');
         $this->assertEquals($other, $event->other);

calendar/tests/externallib_test.php

@@ -104,7 +104,14 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase {
             }
         }
         if (!isset($prop->courseid)) {
-            $prop->courseid = $SITE->id;
+            // Set a default value of the event's course ID field.
+            if ($type === 'user') {
+                // If it's a user event, course ID should be zero.
+                $prop->courseid = 0;
+            } else {
+                // Otherwise, default to the site ID.
+                $prop->courseid = $SITE->id;
+            }
         }
 
         // Determine event priority.
@@ -202,7 +209,7 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase {
         $record = new stdClass();
         $record->courseid = $course->id;
         $courseevent = $this->create_calendar_event('course', $USER->id, 'course', 3, time(), $record);
-        $userevent = $this->create_calendar_event('user', $USER->id);
+        $userevent = $this->create_calendar_event('user', $user->id);
         $record = new stdClass();
         $record->courseid = $course->id;
         $record->groupid = $group->id;
@@ -1977,8 +1984,7 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase {
         $this->resetAfterTest(true);
         $this->setUser($user);
 
-        $this->expectException('moodle_exception');
-
+        $this->expectException(moodle_exception::class);
         external_api::clean_returnvalue(
             core_calendar_external::submit_create_update_form_returns(),
             core_calendar_external::submit_create_update_form($querystring)
@@ -2780,13 +2786,217 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase {
         $this->assertEquals($data['event']['id'], $courseevent->id);
         // User not enrolled in the course cannot load the course event.
         $this->setUser($user2);
-        $this->expectException('required_capability_exception');
+        $this->expectException(moodle_exception::class);
         $data = external_api::clean_returnvalue(
             core_calendar_external::get_calendar_event_by_id_returns(),
             core_calendar_external::get_calendar_event_by_id($courseevent->id)
         );
     }
 
+    /**
+     * User data for testing reading calendar events.
+     *
+     * @return array
+     */
+    public function test_get_calendar_event_by_id_prevent_read_other_users_events_data_provider(): array {
+        $syscontext = context_system::instance();
+        $managerrole = 'manager';
+        return [
+            [true, false, $syscontext, $managerrole, true],
+            [false, false, $syscontext, $managerrole, false],
+            [false, false, null, null, true],
+            [false, true, null, null, false],
+        ];
+    }
+
+    /**
+     * Prevent user from reading other user's event.
+     *
+     * @covers \core_calendar_external::get_calendar_event_by_id
+     * @dataProvider test_get_calendar_event_by_id_prevent_read_other_users_events_data_provider
+     *
+     * @param bool          $isadminevent      Is admin's event
+     * @param bool          $isadmin           Is current user admin user
+     * @param null|stdClass $readerrolecontext Reader role context
+     * @param null|string   $readerrolename    Role name
+     * @param bool          $expectexception   Should the test throw exception
+     */
+    public function test_get_calendar_event_by_id_prevent_read_other_users_events(
+            bool $isadminevent, bool $isadmin, ?stdClass $readerrolecontext,
+            ?string $readerrolename, bool $expectexception) {
+        global $USER, $DB;
+
+        $this->resetAfterTest();
+        $generator = $this->getDataGenerator();
+
+        if ($isadminevent) {
+            $this->setAdminUser();
+        } else {
+            $user = $generator->create_user();
+            $this->setUser($user);
+        }
+        $userevent = $this->create_calendar_event('user event', $USER->id, 'user', 0, time());
+        $results = external_api::clean_returnvalue(
+            core_calendar_external::get_calendar_event_by_id_returns(),
+            core_calendar_external::get_calendar_event_by_id($userevent->id)
+        );
+        $event = reset($results);
+        $this->assertEquals($userevent->id, $event['id']);
+
+        if ($isadmin) {
+            $this->setAdminUser();
+        } else {
+            $reader = $generator->create_user();
+            if ($readerrolename && $readerrolecontext) {
+                $managerroleid = $DB->get_field('role', 'id', ['shortname' => $readerrolename]);
+                role_assign($managerroleid, $reader->id, $readerrolecontext->id);
+            }
+            $this->setUser($reader);
+        }
+
+        if ($expectexception) {
+            // Setup if exception is expected for the test.
+            $this->expectException(moodle_exception::class);
+        }
+        external_api::clean_returnvalue(
+            core_calendar_external::get_calendar_event_by_id_returns(),
+            core_calendar_external::get_calendar_event_by_id($userevent->id)
+        );
+    }
+
+    /**
+     * User data for testing editing or deleting calendar events.
+     *
+     * @return array
+     */
+    public function test_edit_or_delete_other_users_events_data_provider(): array {
+        $syscontext = context_system::instance();
+        $managerrole = 'manager';
+        return [
+            [false, false, $syscontext, $managerrole, false],
+            [false, true, $syscontext, $managerrole, true],
+            [false, false, null, null, true],
+            [true, false, null, null, false],
+        ];
+    }
+
+    /**
+     * Test the behavior of deleting other users' user events.
+     *
+     * @dataProvider test_edit_or_delete_other_users_events_data_provider
+     * @covers \core_calendar_external::delete_calendar_events
+     * @param bool          $isadmin Whether the current user is admin.
+     * @param bool          $isadminevent Whether it's an admin event or not.
+     * @param stdClass|null $writerrolecontext The reader role context.
+     * @param string|null   $writerrolename The role name.
+     * @param bool          $expectexception Whether the test should throw an exception or not.
+     */
+    public function test_delete_other_users_events(bool $isadmin, bool $isadminevent,
+            ?stdClass $writerrolecontext, ?string $writerrolename, bool $expectexception) {
+        global $DB, $USER;
+
+        $this->resetAfterTest();
+        $generator = $this->getDataGenerator();
+
+        if ($isadminevent) {
+            $this->setAdminUser();
+            $user = $USER;
+        } else {
+            $user = $generator->create_user();
+            $this->setUser($user);
+        }
+        $userevent = $this->create_calendar_event('user event', $user->id, 'user', 0, time());
+
+        if ($isadmin) {
+            $this->setAdminUser();
+        } else {
+            $writer = $generator->create_user();
+            if ($writerrolename && $writerrolecontext) {
+                $managerroleid = $DB->get_field('role', 'id', ['shortname' => $writerrolename]);
+                role_assign($managerroleid, $writer->id, $writerrolecontext->id);
+            }
+            $this->setUser($writer);
+        }
+
+        if ($expectexception) {
+            $this->expectException(moodle_exception::class);
+        }
+        $events = [
+            ['eventid' => $userevent->id, 'repeat' => 0]
+        ];
+        core_calendar_external::delete_calendar_events($events);
+    }
+
+    /**
+     * Test the behavior of editing other users' user events
+     *
+     * @dataProvider test_edit_or_delete_other_users_events_data_provider
+     * @covers \core_calendar_external::submit_create_update_form
+     * @param bool          $isadmin Whether the current user is admin.
+     * @param bool          $isadminevent Whether it's an admin event or not.
+     * @param stdClass|null $writerrolecontext The reader role context.
+     * @param string|null   $writerrolename The role name.
+     * @param bool          $expectexception Whether the test should throw an exception or not.
+     */
+    public function test_edit_other_users_events(bool $isadmin, bool $isadminevent,
+            ?stdClass $writerrolecontext, ?string $writerrolename, bool $expectexception) {
+        global $DB, $USER;
+
+        $this->resetAfterTest();
+
+        $generator = $this->getDataGenerator();
+        if ($isadminevent) {
+            $this->setAdminUser();
+            $user = $USER;
+        } else {
+            $user = $generator->create_user();
+        }
+
+        $formdata = [
+            'id' => 0,
+            'userid' => $user->id,
+            'modulename' => '',
+            'instance' => 0,
+            'visible' => 1,
+            'eventtype' => 'user',
+            'name' => 'Test',
+            'timestart' => [
+                'day' => 1,
+                'month' => 1,
+                'year' => 2021,
+                'hour' => 1,
+                'minute' => 0,
+            ],
+            'description' => [
+                'text' => 'xxxxx',
+                'format' => 1,
+                'itemid' => 0
+            ],
+            'location' => 'Test',
+            'duration' => 0,
+        ];
+        $formdata = \core_calendar\local\event\forms\create::mock_generate_submit_keys($formdata);
+
+        $querystring = http_build_query($formdata, '', '&');
+
+        if ($isadmin) {
+            $this->setAdminUser();
+        } else {
+            $writer = $generator->create_user();
+            if ($writerrolename && $writerrolecontext) {
+                $managerroleid = $DB->get_field('role', 'id', ['shortname' => $writerrolename]);
+                role_assign($managerroleid, $writer->id, $writerrolecontext->id);
+            }
+            $this->setUser($writer);
+        }
+        $USER->ignoresesskey = true;
+
+        if ($expectexception) {
+            $this->expectException(moodle_exception::class);
+        }
+        core_calendar_external::submit_create_update_form($querystring);
+    }
+
     /**
      * A user should not be able load the calendar events for a category they cannot see.
      */

calendar/tests/lib_test.php

@@ -997,4 +997,53 @@ class lib_test extends \advanced_testcase {
 
         $this->assertEquals($expected, $authtoken);
     }
+
+    /**
+     *  Test calendar_can_manage_user_event for different users.
+     *
+     * @covers ::calendar_can_manage_user_event
+     */
+    public function test_calendar_can_manage_user_event() {
+        global $DB, $USER;
+        $generator = $this->getDataGenerator();
+        $sitecontext = \context_system::instance();
+        $this->resetAfterTest();
+        $this->setAdminUser();
+        $user1 = $generator->create_user();
+        $user2 = $generator->create_user();
+        $adminevent = create_event([
+            'eventtype' => 'user',
+            'userid' => $USER->id,
+        ]);
+
+        $this->setUser($user1);
+        $user1event = create_event([
+            'name' => 'user1 event',
+            'eventtype' => 'user',
+            'userid' => $user1->id,
+        ]);
+        $this->setUser($user2);
+        $user2event = create_event([
+            'name' => 'user2 event',
+            'eventtype' => 'user',
+            'userid' => $user2->id,
+        ]);
+        $this->setUser($user1);
+        $result = calendar_can_manage_user_event($user1event);
+        $this->assertEquals(true, $result);
+        $result = calendar_can_manage_user_event($user2event);
+        $this->assertEquals(false, $result);
+
+        $sitemanager = $generator->create_user();
+
+        $managerroleid = $DB->get_field('role', 'id', ['shortname' => 'manager']);
+        role_assign($managerroleid, $sitemanager->id, $sitecontext->id);
+
+        $this->setUser($sitemanager);
+
+        $result = calendar_can_manage_user_event($user1event);
+        $this->assertEquals(true, $result);
+        $result = calendar_can_manage_user_event($adminevent);
+        $this->assertEquals(false, $result);
+    }
 }

calendar/upgrade.txt

@@ -7,6 +7,7 @@ information provided here is intended especially for developers.
   - core_calendar_external::get_calendar_action_events_by_courses
   - core_calendar_external::get_calendar_action_events_by_course
 * Added core_calendar_delete_subscription, which allows to delete the calendar subscription.
+* Updated calendar_can_manage_user_event() function to check permissions to user events.
 * The following functions have been deprecated because they were no longer used:
   - calendar_process_subscription_row()
   - calendar_import_icalendar_events()

lang/en/error.php

@@ -464,6 +464,7 @@ $string['nopermissiontorate'] = 'Rating of items not allowed!';
 $string['nopermissiontoshow'] = 'No permission to see this!';
 $string['nopermissiontounlock'] = 'No permission to unlock!';
 $string['nopermissiontoupdatecalendar'] = 'Sorry, but you do not have permission to update the calendar event.';
+$string['nopermissiontoviewcalendar'] = 'Sorry, but you do not have permission to view the calendar event.';
 $string['nopermissiontoviewgrades'] = 'Cannot view grades.';
 $string['nopermissiontoviewletergrade'] = 'Missing permission to view letter grades';
 $string['nopermissiontoviewpage'] = 'You are not allowed to look at this page';