From afd4ccf425c2bba6ae77658f9ebca55f4bd1afbb Mon Sep 17 00:00:00 2001 From: Laurent David Date: Tue, 23 Jan 2024 09:11:12 +0100 Subject: [PATCH] MDL-80565 report_log: Prevent user from seeing other logs * When no groups are selected we forcefully prevent users from seeing users' logs from other groups --- report/log/classes/renderable.php | 22 +++-------- report/log/classes/table_log.php | 38 +++++++++++++++---- report/log/tests/renderable_test.php | 57 ++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 25 deletions(-) diff --git a/report/log/classes/renderable.php b/report/log/classes/renderable.php index a7e28132a84..34ce7dfb60f 100644 --- a/report/log/classes/renderable.php +++ b/report/log/classes/renderable.php @@ -258,26 +258,14 @@ class report_log_renderable implements renderable { // Setup for group handling. $groupmode = groups_get_course_groupmode($this->course); if ($groupmode == SEPARATEGROUPS and !has_capability('moodle/site:accessallgroups', $context)) { - $selectedgroup = -1; - } else if ($groupmode) { - $selectedgroup = $this->groupid; - } else { - $selectedgroup = 0; - } - - if ($selectedgroup === -1) { if (isset($SESSION->currentgroup[$this->course->id])) { $selectedgroup = $SESSION->currentgroup[$this->course->id]; - } else { - $selectedgroup = groups_get_all_groups($this->course->id, $USER->id); - if (is_array($selectedgroup)) { - $groupids = array_keys($selectedgroup); - $selectedgroup = array_shift($groupids); - $SESSION->currentgroup[$this->course->id] = $selectedgroup; - } else { - $selectedgroup = 0; - } + } else if ($this->groupid > 0) { + $SESSION->currentgroup[$this->course->id] = $this->groupid; + $selectedgroup = $this->groupid; } + } else if ($groupmode) { + $selectedgroup = $this->groupid; } return $selectedgroup; } diff --git a/report/log/classes/table_log.php b/report/log/classes/table_log.php index e64dfad0267..4cb1da64d1b 100644 --- a/report/log/classes/table_log.php +++ b/report/log/classes/table_log.php @@ -23,7 +23,8 @@ */ defined('MOODLE_INTERNAL') || die; - +global $CFG; +require_once($CFG->libdir . '/tablelib.php'); /** * Table log class for displaying logs. * @@ -403,7 +404,7 @@ class report_log_table_log extends table_sql { * @param bool $useinitialsbar do you want to use the initials bar. */ public function query_db($pagesize, $useinitialsbar = true) { - global $DB; + global $DB, $USER; $joins = array(); $params = array(); @@ -438,14 +439,35 @@ class report_log_table_log extends table_sql { } // Getting all members of a group. - if ($groupid and empty($this->filterparams->userid)) { - if ($gusers = groups_get_members($groupid)) { - $gusers = array_keys($gusers); - $joins[] = 'userid IN (' . implode(',', $gusers) . ')'; + if (empty($this->filterparams->userid)) { + if ($groupid) { + if ($gusers = groups_get_members($groupid)) { + $gusers = array_keys($gusers); + $joins[] = 'userid IN (' . implode(',', $gusers) . ')'; + } else { + $joins[] = 'userid = 0'; // No users in groups, so we want something that will always be false. + } } else { - $joins[] = 'userid = 0'; // No users in groups, so we want something that will always be false. + // No group selected and we are not filtering by user, so we want all users that are visible to the current user. + // If we are in a course, then let's check what logs we can see. + $course = get_course($this->filterparams->courseid); + $groupmode = groups_get_course_groupmode($course); + $context = context_course::instance($this->filterparams->courseid); + $userid = 0; + if ($groupmode == SEPARATEGROUPS && !has_capability('moodle/site:accessallgroups', $context)) { + $userid = $USER->id; + } + $cgroups = groups_get_all_groups($this->filterparams->courseid, $userid); + $cgroups = array_keys($cgroups); + if ($groupmode != SEPARATEGROUPS || has_capability('moodle/site:accessallgroups', $context)) { + $cgroups[] = USERSWITHOUTGROUP; + } + // If that's the case, limit the users to be in the groups only, defined by the filter. + [$groupmembersql, $groupmemberparams] = groups_get_members_ids_sql($cgroups, $context); + $joins[] = "userid IN ($groupmembersql)"; + $params = array_merge($params, $groupmemberparams); } - } else if (!empty($this->filterparams->userid)) { + } else { $joins[] = "userid = :userid"; $params['userid'] = $this->filterparams->userid; } diff --git a/report/log/tests/renderable_test.php b/report/log/tests/renderable_test.php index b3a25d70e16..5dd39cee406 100644 --- a/report/log/tests/renderable_test.php +++ b/report/log/tests/renderable_test.php @@ -380,4 +380,61 @@ class renderable_test extends \advanced_testcase { $groups = $renderable->get_group_list(); $this->assertCount($expectedcount, $groups); } + + /** + * Test table_log + * + * @param int $courseindex + * @param string $username + * @param array $expectedusers + * @param string|null $groupname + * @covers \report_log_renderable::get_user_list + * @dataProvider get_user_visibility_list_provider + * @return void + */ + public function test_get_table_logs(int $courseindex, string $username, array $expectedusers, ?string $groupname = null): void { + global $DB, $PAGE; + $this->preventResetByRollback(); // This is to ensure that we can actually trigger event and record them in the log store. + // Configure log store. + set_config('enabled_stores', 'logstore_standard', 'tool_log'); + $manager = get_log_manager(true); + $DB->delete_records('logstore_standard_log'); + + foreach ($this->courses as $course) { + foreach ($this->users as $user) { + $eventdata = [ + 'context' => context_course::instance($course->id), + 'userid' => $user->id, + ]; + $event = \core\event\course_viewed::create($eventdata); + $event->trigger(); + } + } + $stores = $manager->get_readers(); + $store = $stores['logstore_standard']; + // Build the report. + $currentuser = $this->users[$username]; + $this->setUser($currentuser->id); + $groupid = 0; + if ($groupname) { + $groupid = $this->groupsbycourse[$courseindex][$groupname]->id; + } + $PAGE->set_url('/report/log/index.php?id=' . $this->courses[$courseindex]->id); + $renderable = new \report_log_renderable("", (int) $this->courses[$courseindex]->id, 0, 0, '', $groupid); + $renderable->setup_table(); + $table = $renderable->tablelog; + $store->flush(); + $table->query_db(100); + $usernames = []; + foreach ($table->rawdata as $event) { + if (get_class($event) !== \core\event\course_viewed::class) { + continue; + } + $user = core_user::get_user($event->userid, '*', MUST_EXIST); + $usernames[] = $user->username; + } + sort($expectedusers); + sort($usernames); + $this->assertEquals($expectedusers, $usernames); + } }