From 943c33c3f72798aada1102a5ab3ed676631cdf6d Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Tue, 31 Oct 2017 11:25:48 +0800 Subject: [PATCH 1/2] MDL-60626 calendar: Check capabilities for api All external functions must check all capabilities before returning data. The calendar API itself does not check capabilities (I wish it did), so we must be careful exposing these functions to webservices. --- calendar/externallib.php | 48 ++++++++++++++++++++++++++++++--------- calendar/lib.php | 59 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 11 deletions(-) diff --git a/calendar/externallib.php b/calendar/externallib.php index 0c82f8a..3d0054d 100644 --- a/calendar/externallib.php +++ b/calendar/externallib.php @@ -203,6 +203,8 @@ class core_calendar_external extends external_api { $courses = $params['events']['courseids']; $funcparam['courses'] = $courses; } + // Now get categories we can get events from. + $categories = \coursecat::get_all(); // Let us findout groups that we can return events from. if (!$hassystemcap) { @@ -273,7 +275,8 @@ class core_calendar_external extends external_api { } else { // Can the user actually see this event? $eventobj = calendar_event::load($eventobj); - if (($eventobj->courseid == $SITE->id) || + if ((($eventobj->courseid == $SITE->id) && (empty($eventobj->categoryid))) || + (!empty($eventobj->categoryid) && in_array($eventobj->categoryid, $categories)) || (!empty($eventobj->groupid) && in_array($eventobj->groupid, $groups)) || (!empty($eventobj->courseid) && in_array($eventobj->courseid, $courses)) || ($USER->id == $eventobj->userid) || @@ -724,6 +727,14 @@ class core_calendar_external extends external_api { $warnings = array(); $legacyevent = calendar_event::load($eventid); + // Must check we can see this event. + if (!calendar_view_event_allowed($legacyevent)) { + // 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', ''); + } + $legacyevent->count_repeats(); $eventmapper = event_container::get_event_mapper(); @@ -905,9 +916,14 @@ class core_calendar_external extends external_api { $PAGE->set_url('/calendar/'); if ($courseid != SITEID && !empty($courseid)) { - // Course ID must be valid and existing. - $course = $DB->get_record('course', array('id' => $courseid), '*', MUST_EXIST); - $courses = [$course->id => $course]; + // Course ID must be valid and existing (and you must be allowed to see it). + $mycourses = calendar_get_default_courses($courseid, '*', true); + if (!isset($mycourses[$courseid])) { + $coursecontext = context_course::instance($courseid); + throw new \required_capability_exception($coursecontext, 'moodle/course:view', 'nopermission', ''); + } + $course = $mycourses[$courseid]; + $courses = [$courseid => $course]; $coursecat = \coursecat::get($course->category); $category = $coursecat->get_db_record(); @@ -995,9 +1011,14 @@ class core_calendar_external extends external_api { ]); if ($courseid != SITEID && !empty($courseid)) { - // Course ID must be valid and existing. - $course = $DB->get_record('course', array('id' => $courseid), '*', MUST_EXIST); - $courses = [$course->id => $course]; + // Course ID must be valid and existing (and you must be allowed to see it). + $mycourses = calendar_get_default_courses($courseid, '*', true); + if (!isset($mycourses[$courseid])) { + $coursecontext = context_course::instance($courseid); + throw new \required_capability_exception($coursecontext, 'moodle/course:view', 'nopermission', ''); + } + $course = $mycourses[$courseid]; + $courses = [$courseid => $course]; $coursecat = \coursecat::get($course->category); $category = $coursecat->get_db_record(); @@ -1029,7 +1050,7 @@ class core_calendar_external extends external_api { $calendar = new calendar_information(0, 0, 0, $time); $calendar->set_sources($course, $courses, $category); - list($data, $template) = calendar_get_view($calendar, 'day', $params['includenavigation']); + list($data, $template) = calendar_get_view($calendar, 'day'); return $data; } @@ -1157,9 +1178,14 @@ class core_calendar_external extends external_api { ]); if ($courseid != SITEID && !empty($courseid)) { - // Course ID must be valid and existing. - $course = $DB->get_record('course', array('id' => $courseid), '*', MUST_EXIST); - $courses = [$course->id => $course]; + // Course ID must be valid and existing (and you must be allowed to see it). + $mycourses = calendar_get_default_courses($courseid, '*', true); + if (!isset($mycourses[$courseid])) { + $coursecontext = context_course::instance($courseid); + throw new \required_capability_exception($coursecontext, 'moodle/course:view', 'nopermission', ''); + } + $course = $mycourses[$courseid]; + $courses = [$courseid => $course]; } else { $course = get_site(); $courses = calendar_get_default_courses(); diff --git a/calendar/lib.php b/calendar/lib.php index 8c20f9a..f4cf5f2 100644 --- a/calendar/lib.php +++ b/calendar/lib.php @@ -2030,6 +2030,65 @@ function calendar_set_filters(array $courseeventsfrom, $ignorefilters = false) { } /** + * Return the capability for viewing a calendar event. + * + * @param calendar_event $event event object + * @return boolean + */ +function calendar_view_event_allowed($event) { + global $USER; + + // Anyone can see site events. + if ($event->courseid && $event->courseid == SITEID) { + 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)) { + return true; + } + + + if (!empty($event->groupid)) { + // If it is a group event we need to be able to manage events in the course, or be in the group. + return has_capability('moodle/calendar:manageentries', $event->context) + || groups_is_member($event->groupid); + } else if ($event->modulename) { + // If this is a module event we need to be able to see the module. + $coursemodules = get_fast_modinfo($event->courseid)->instances; + $hasmodule = isset($coursemodules[$event->modulename]); + $hasinstance = isset($coursemodules[$event->modulename][$event->instance]); + + // If modinfo doesn't know about the module, return false to be safe. + if (!$hasmodule || !$hasinstance) { + return false; + } + + $cm = $coursemodules[$event->modulename][$event->instance]; + return $cm->uservisible; + } else if ($event->categoryid) { + // If this is a category we need to be able to see the category. + $cat = \coursecat::get($event->categoryid, IGNORE_MISSING); + if (!$cat) { + return false; + } + } else if (!empty($event->courseid)) { + // If it is a course event we need to be able to manage events in the course, or be in the course. + $mycourses = enrol_get_my_courses('id'); + return has_capability('moodle/calendar:manageentries', $event->context) || isset($mycourses[$event->courseid]); + } else if ($event->userid && $event->userid != $USER->id) { + // No-one can ever see another users events. + return false; + } else { + throw new moodle_exception('unknown event type'); + } + + return false; +} + +/** * Return the capability for editing calendar event. * * @param calendar_event $event event object -- 2.7.4 From 0c9b972eda44c301cad588dcea69a4e08cdc862f Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Tue, 31 Oct 2017 11:27:27 +0800 Subject: [PATCH 2/2] MDL-60626 calendar: Unit tests for capabilities Check various calendar external functions for capabilities. --- calendar/tests/externallib_test.php | 217 ++++++++++++++++++++++++++++++++++++ 1 file changed, 217 insertions(+) diff --git a/calendar/tests/externallib_test.php b/calendar/tests/externallib_test.php index f60d57f..14bbae2 100644 --- a/calendar/tests/externallib_test.php +++ b/calendar/tests/externallib_test.php @@ -2201,4 +2201,221 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase { $this->assertTrue($result['validationerror']); } + + /** + * A user should not be able load the calendar monthly view for a course they cannot access. + */ + public function test_get_calendar_monthly_view_no_course_permission() { + global $USER; + $this->resetAfterTest(true); + $this->setAdminUser(); + + $generator = $this->getDataGenerator(); + $user1 = $generator->create_user(); + $user2 = $generator->create_user(); + $course = $generator->create_course(); + $context = context_course::instance($course->id); + $generator->enrol_user($user1->id, $course->id, 'student'); + $properties = []; + $name = 'Course Event (course' . $course->id . ')'; + $record = new stdClass(); + $record->courseid = $course->id; + $courseevent = $this->create_calendar_event($name, $USER->id, 'course', 0, time(), $record); + + $timestart = new DateTime(); + // Admin can load the course. + $data = external_api::clean_returnvalue( + core_calendar_external::get_calendar_monthly_view_returns(), + core_calendar_external::get_calendar_monthly_view($timestart->format('n'), $timestart->format('Y'), + $course->id, null, false) + ); + $this->assertEquals($data['courseid'], $course->id); + // User enrolled in the course can load the course calendar. + $this->setUser($user1); + $data = external_api::clean_returnvalue( + core_calendar_external::get_calendar_monthly_view_returns(), + core_calendar_external::get_calendar_monthly_view($timestart->format('n'), $timestart->format('Y'), + $course->id, null, false) + ); + $this->assertEquals($data['courseid'], $course->id); + // User not enrolled in the course cannot load the course calendar. + $this->setUser($user2); + $this->setExpectedException('required_capability_exception'); + $data = external_api::clean_returnvalue( + core_calendar_external::get_calendar_monthly_view_returns(), + core_calendar_external::get_calendar_monthly_view($timestart->format('n'), $timestart->format('Y'), + $course->id, null, false) + ); + } + + /** + * A user should not be able load the calendar day view for a course they cannot access. + */ + public function test_get_calendar_day_view_no_course_permission() { + global $USER; + $this->resetAfterTest(true); + $this->setAdminUser(); + + $generator = $this->getDataGenerator(); + $user1 = $generator->create_user(); + $user2 = $generator->create_user(); + $course = $generator->create_course(); + $context = context_course::instance($course->id); + $generator->enrol_user($user1->id, $course->id, 'student'); + $properties = []; + $name = 'Course Event (course' . $course->id . ')'; + $record = new stdClass(); + $record->courseid = $course->id; + $courseevent = $this->create_calendar_event($name, $USER->id, 'course', 0, time(), $record); + + $timestart = new DateTime(); + // Admin can load the course. + $data = external_api::clean_returnvalue( + core_calendar_external::get_calendar_day_view_returns(), + core_calendar_external::get_calendar_day_view($timestart->format('n'), $timestart->format('Y'), + $timestart->format('j'), $course->id, null) + ); + $this->assertEquals($data['courseid'], $course->id); + // User enrolled in the course can load the course calendar. + $this->setUser($user1); + $data = external_api::clean_returnvalue( + core_calendar_external::get_calendar_day_view_returns(), + core_calendar_external::get_calendar_day_view($timestart->format('n'), $timestart->format('Y'), + $timestart->format('j'), $course->id, null) + ); + $this->assertEquals($data['courseid'], $course->id); + // User not enrolled in the course cannot load the course calendar. + $this->setUser($user2); + $this->setExpectedException('required_capability_exception'); + $data = external_api::clean_returnvalue( + core_calendar_external::get_calendar_day_view_returns(), + core_calendar_external::get_calendar_day_view($timestart->format('n'), $timestart->format('Y'), + $timestart->format('j'), $course->id, null) + ); + } + + /** + * A user should not be able load the calendar upcoming view for a course they cannot access. + */ + public function test_get_calendar_upcoming_view_no_course_permission() { + global $USER; + $this->resetAfterTest(true); + $this->setAdminUser(); + + $generator = $this->getDataGenerator(); + $user1 = $generator->create_user(); + $user2 = $generator->create_user(); + $course = $generator->create_course(); + $context = context_course::instance($course->id); + $generator->enrol_user($user1->id, $course->id, 'student'); + $properties = []; + $name = 'Course Event (course' . $course->id . ')'; + $record = new stdClass(); + $record->courseid = $course->id; + $courseevent = $this->create_calendar_event($name, $USER->id, 'course', 0, time(), $record); + + $timestart = new DateTime(); + // Admin can load the course. + $data = external_api::clean_returnvalue( + core_calendar_external::get_calendar_upcoming_view_returns(), + core_calendar_external::get_calendar_upcoming_view($course->id) + ); + $this->assertEquals($data['courseid'], $course->id); + // User enrolled in the course can load the course calendar. + $this->setUser($user1); + $data = external_api::clean_returnvalue( + core_calendar_external::get_calendar_upcoming_view_returns(), + core_calendar_external::get_calendar_upcoming_view($course->id) + ); + $this->assertEquals($data['courseid'], $course->id); + // User not enrolled in the course cannot load the course calendar. + $this->setUser($user2); + $this->setExpectedException('required_capability_exception'); + $data = external_api::clean_returnvalue( + core_calendar_external::get_calendar_upcoming_view_returns(), + core_calendar_external::get_calendar_upcoming_view($course->id) + ); + } + + /** + * A user should not be able load the calendar event for a course they cannot access. + */ + public function test_get_calendar_event_by_id_no_course_permission() { + global $USER; + $this->resetAfterTest(true); + $this->setAdminUser(); + + $generator = $this->getDataGenerator(); + $user1 = $generator->create_user(); + $user2 = $generator->create_user(); + $course = $generator->create_course(); + $context = context_course::instance($course->id); + $generator->enrol_user($user1->id, $course->id, 'student'); + $properties = []; + $name = 'Course Event (course' . $course->id . ')'; + $record = new stdClass(); + $record->courseid = $course->id; + $courseevent = $this->create_calendar_event($name, $USER->id, 'course', 0, time(), $record); + + $timestart = new DateTime(); + // Admin can load the course event. + $data = external_api::clean_returnvalue( + core_calendar_external::get_calendar_event_by_id_returns(), + core_calendar_external::get_calendar_event_by_id($courseevent->id) + ); + $this->assertEquals($data['event']['id'], $courseevent->id); + // User enrolled in the course can load the course event. + $this->setUser($user1); + $data = external_api::clean_returnvalue( + core_calendar_external::get_calendar_event_by_id_returns(), + core_calendar_external::get_calendar_event_by_id($courseevent->id) + ); + $this->assertEquals($data['event']['id'], $courseevent->id); + // User not enrolled in the course cannot load the course event. + $this->setUser($user2); + $this->setExpectedException('required_capability_exception'); + $data = external_api::clean_returnvalue( + core_calendar_external::get_calendar_event_by_id_returns(), + core_calendar_external::get_calendar_event_by_id($courseevent->id) + ); + } + + /** + * A user should not be able load the calendar events for a category they cannot see. + */ + public function test_get_calendar_events_hidden_category() { + global $USER; + $this->resetAfterTest(true); + $this->setAdminUser(); + + $generator = $this->getDataGenerator(); + $user1 = $generator->create_user(); + $category = $generator->create_category(['visible' => 0]); + $context = context_coursecat::instance($category->id); + $properties = []; + $name = 'Category Event (category: ' . $category->id . ')'; + $record = new stdClass(); + $record->categoryid = $category->id; + $categoryevent = $this->create_calendar_event($name, $USER->id, 'category', 0, time(), $record); + + $timestart = new DateTime(); + $events = [ + 'eventids' => [$categoryevent->id] + ]; + $options = []; + // Admin can load the category event. + $data = external_api::clean_returnvalue( + core_calendar_external::get_calendar_events_returns(), + core_calendar_external::get_calendar_events($events, $options) + ); + $this->assertEquals($data['events'][0]['id'], $categoryevent->id); + // User with no special permission to see hidden categories will not see the event. + $this->setUser($user1); + $data = external_api::clean_returnvalue( + core_calendar_external::get_calendar_events_returns(), + core_calendar_external::get_calendar_events($events, $options) + ); + $this->assertCount(0, $data['events']); + $this->assertEquals('nopermissions', $data['warnings'][0]['warningcode']); + } } -- 2.7.4