mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-04-13 20:42:22 +02:00
Code Issues Projects Releases Wiki Activity

MDL-68612 user: Update participants group filtering to enforce groups

Browse Source 
This is required to ensure regardless of user applied filters, only
members of groups visible to the user are ever fetched. This also
includes a fix to remove the groups filter option where no groups
mode is applied.
This commit is contained in:
Michael Hawkins 2020-05-27 18:49:18 +08:00
parent d53dd31f05
commit d85315ee8c
3 changed files with 452 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
user/classes/output/participants_filter.php
View File

@ -210,7 +210,8 @@ class participants_filter implements renderable, templatable {
$groups = groups_get_all_groups($this->course->id, $USER->id);
}
if (empty($groups)) {
// Return no data if no groups found (which includes if the only value is 'No group').
if (empty($groups) || (count($groups) === 1 && array_key_exists(-1, $groups))) {
return null;
}

52
user/classes/table/participants_search.php
View File

@ -314,6 +314,8 @@ class participants_search {
* @return array SQL query data in the format ['sql' => '', 'forcedsql' => '', 'params' => []].
*/
protected function get_enrolled_sql(): array {
global $USER;
$isfrontpage = ($this->context->instanceid == SITEID);
$prefix = 'eu_';
$filteruid = "{$prefix}u.id";
@ -357,15 +359,43 @@ class participants_search {
$params = array_merge($params, $methodparams, $statusparams);
}
// Prepare any groups filtering.
$groupids = [];
if ($this->filterset->has_filter('groups')) {
$groupids = $this->filterset->get_filter('groups')->get_filter_values();
}
// Force additional groups filtering if required due to lack of capabilities.
// Note: This means results will always be limited to allowed groups, even if the user applies their own groups filtering.
$canaccessallgroups = has_capability('moodle/site:accessallgroups', $this->context);
$forcegroups = ($this->course->groupmode == SEPARATEGROUPS && !$canaccessallgroups);
if ($forcegroups) {
$allowedgroupids = array_keys(groups_get_all_groups($this->course->id, $USER->id));
// Users not in any group in a course with separate groups mode should not be able to access the participants filter.
if (empty($allowedgroupids)) {
// The UI does not support this, so it should not be reachable unless someone is trying to bypass the restriction.
throw new \coding_exception('User must be part of a group to filter by participants.');
}
$forceduid = "{$forcedprefix}u.id";
$forcedjointype = $this->get_groups_jointype(\core_table\local\filter\filter::JOINTYPE_ANY);
$forcedgroupjoin = groups_get_members_join($allowedgroupids, $forceduid, $this->context, $forcedjointype);
$forcedjoins[] = $forcedgroupjoin->joins;
$forcedwhere .= "AND ({$forcedgroupjoin->wheres})";
$params = array_merge($params, $forcedgroupjoin->params);
// Remove any filtered groups the user does not have access to.
$groupids = array_intersect($allowedgroupids, $groupids);
}
// Prepare any user defined groups filtering.
if ($groupids) {
$groupjoin = groups_get_members_join($groupids, $filteruid, $this->context, $this->get_groups_jointype());
$joins[] = $groupjoin->joins;
$params = array_merge($params, $groupjoin->params);
if (!empty($groupjoin->wheres)) {
@ -685,12 +715,28 @@ class participants_search {
* Fetch the groups filter's grouplib jointype, based on its filterset jointype.
* This mapping is to ensure compatibility between the two, should their values ever differ.
*
* @param int|null $forcedjointype If set, specifies the join type to fetch mapping for (used when applying forced filtering).
* If null, then user defined filter join type is used.
* @return int
*/
protected function get_groups_jointype(): int {
protected function get_groups_jointype(?int $forcedjointype = null): int {
// If applying forced groups filter and no manual groups filtering is applied, add an empty filter so we can map the join.
if (!is_null($forcedjointype) && !$this->filterset->has_filter('groups')) {
$this->filterset->add_filter(new \core_table\local\filter\integer_filter('groups'));
}
$groupsfilter = $this->filterset->get_filter('groups');
switch ($groupsfilter->get_join_type()) {
if (is_null($forcedjointype)) {
// Fetch join type mapping for a user supplied groups filtering.
$filterjointype = $groupsfilter->get_join_type();
} else {
// Fetch join type mapping for forced groups filtering.
$filterjointype = $forcedjointype;
}
switch ($filterjointype) {
case $groupsfilter::JOINTYPE_NONE:
$groupsjoin = GROUPS_JOIN_NONE;
break;

401
user/tests/table/participants_search_test.php
View File

@ -2008,6 +2008,407 @@ class participants_search_test extends advanced_testcase {
return $finaltests;
}
/**
* Ensure that the groups filter works as expected when separate groups mode is enabled, with the provided test cases.
*
* @param array $usersdata The list of users to create
* @param array $groupsavailable The names of groups that should be created in the course
* @param array $filtergroups The names of groups to filter by
* @param int $jointype The join type to use when combining filter values
* @param int $count The expected count
* @param array $expectedusers
* @param string $loginusername The user to login as for the tests
* @dataProvider groups_separate_provider
*/
public function test_groups_filter_separate_groups(array $usersdata, array $groupsavailable, array $filtergroups, int $jointype,
int $count, array $expectedusers, string $loginusername): void {
$course = $this->getDataGenerator()->create_course();
$coursecontext = context_course::instance($course->id);
$users = [];
// Enable separate groups mode on the course.
$course->groupmode = SEPARATEGROUPS;
$course->groupmodeforce = true;
update_course($course);
// Prepare data for filtering by users in no groups.
$nogroupsdata = (object) [
'id' => USERSWITHOUTGROUP,
];
// Map group names to group data.
$groupsdata = ['nogroups' => $nogroupsdata];
foreach ($groupsavailable as $groupname) {
$groupinfo = [
'courseid' => $course->id,
'name' => $groupname,
];
$groupsdata[$groupname] = $this->getDataGenerator()->create_group($groupinfo);
}
foreach ($usersdata as $username => $userdata) {
$user = $this->getDataGenerator()->create_user(['username' => $username]);
$this->getDataGenerator()->enrol_user($user->id, $course->id, 'student');
if (array_key_exists('groups', $userdata)) {
foreach ($userdata['groups'] as $groupname) {
$userinfo = [
'userid' => $user->id,
'groupid' => (int) $groupsdata[$groupname]->id,
];
$this->getDataGenerator()->create_group_member($userinfo);
}
}
$users[$username] = $user;
if ($username == $loginusername) {
$loginuser = $user;
}
}
// Create a secondary course with users. We should not see these users.
$this->create_course_with_users(1, 1, 1, 1);
// Log in as the user to be tested.
$this->setUser($loginuser);
// Create the basic filter.
$filterset = new participants_filterset();
$filterset->add_filter(new integer_filter('courseid', null, [(int) $course->id]));
// Create the groups filter.
$groupsfilter = new integer_filter('groups');
$filterset->add_filter($groupsfilter);
// Configure the filter.
foreach ($filtergroups as $filtergroupname) {
$groupsfilter->add_filter_value((int) $groupsdata[$filtergroupname]->id);
}
$groupsfilter->set_join_type($jointype);
// Run the search.
$search = new participants_search($course, $coursecontext, $filterset);
// Tests on user in no groups should throw an exception as they are not supported (participants are not visible to them).
if (in_array('exception', $expectedusers)) {
$this->expectException(\coding_exception::class);
$rs = $search->get_participants();
} else {
// All other cases are tested as normal.
$rs = $search->get_participants();
$this->assertInstanceOf(moodle_recordset::class, $rs);
$records = $this->convert_recordset_to_array($rs);
$this->assertCount($count, $records);
$this->assertEquals($count, $search->get_total_participants_count());
foreach ($expectedusers as $expecteduser) {
$this->assertArrayHasKey($users[$expecteduser]->id, $records);
}
}
}
/**
* Data provider for groups filter tests.
*
* @return array
*/
public function groups_separate_provider(): array {
$tests = [
'Users in different groups with separate groups mode enabled' => (object) [
'groupsavailable' => [
'groupa',
'groupb',
'groupc',
],
'users' => [
'a' => [
'groups' => ['groupa'],
],
'b' => [
'groups' => ['groupb'],
],
'c' => [
'groups' => ['groupa', 'groupb'],
],
'd' => [
'groups' => [],
],
],
'expect' => [
// Tests for jointype: ANY.
'ANY: No filter, user in one group' => (object) [
'loginuser' => 'a',
'groups' => [],
'jointype' => filter::JOINTYPE_ANY,
'count' => 2,
'expectedusers' => [
'a',
'c',
],
],
'ANY: No filter, user in multiple groups' => (object) [
'loginuser' => 'c',
'groups' => [],
'jointype' => filter::JOINTYPE_ANY,
'count' => 3,
'expectedusers' => [
'a',
'b',
'c',
],
],
'ANY: No filter, user in no groups' => (object) [
'loginuser' => 'd',
'groups' => [],
'jointype' => filter::JOINTYPE_ANY,
'count' => 0,
'expectedusers' => ['exception'],
],
'ANY: Filter on a single group, user in one group' => (object) [
'loginuser' => 'a',
'groups' => ['groupa'],
'jointype' => filter::JOINTYPE_ANY,
'count' => 2,
'expectedusers' => [
'a',
'c',
],
],
'ANY: Filter on a single group, user in multple groups' => (object) [
'loginuser' => 'c',
'groups' => ['groupa'],
'jointype' => filter::JOINTYPE_ANY,
'count' => 2,
'expectedusers' => [
'a',
'c',
],
],
'ANY: Filter on a single group, user in no groups' => (object) [
'loginuser' => 'd',
'groups' => ['groupa'],
'jointype' => filter::JOINTYPE_ANY,
'count' => 0,
'expectedusers' => ['exception'],
],
'ANY: Filter on multiple groups, user in one group (ignore invalid groups)' => (object) [
'loginuser' => 'a',
'groups' => ['groupa', 'groupb'],
'jointype' => filter::JOINTYPE_ANY,
'count' => 2,
'expectedusers' => [
'a',
'c',
],
],
'ANY: Filter on multiple groups, user in multiple groups' => (object) [
'loginuser' => 'c',
'groups' => ['groupa', 'groupb'],
'jointype' => filter::JOINTYPE_ANY,
'count' => 3,
'expectedusers' => [
'a',
'b',
'c',
],
],
'ANY: Filter on multiple groups or no groups, user in multiple groups (ignore no groups)' => (object) [
'loginuser' => 'c',
'groups' => ['groupa', 'groupb', 'nogroups'],
'jointype' => filter::JOINTYPE_ANY,
'count' => 3,
'expectedusers' => [
'a',
'b',
'c',
],
],
// Tests for jointype: ALL.
'ALL: No filter, user in one group' => (object) [
'loginuser' => 'a',
'groups' => [],
'jointype' => filter::JOINTYPE_ALL,
'count' => 2,
'expectedusers' => [
'a',
'c',
],
],
'ALL: No filter, user in multiple groups' => (object) [
'loginuser' => 'c',
'groups' => [],
'jointype' => filter::JOINTYPE_ALL,
'count' => 3,
'expectedusers' => [
'a',
'b',
'c',
],
],
'ALL: No filter, user in no groups' => (object) [
'loginuser' => 'd',
'groups' => [],
'jointype' => filter::JOINTYPE_ALL,
'count' => 0,
'expectedusers' => ['exception'],
],
'ALL: Filter on a single group, user in one group' => (object) [
'loginuser' => 'a',
'groups' => ['groupa'],
'jointype' => filter::JOINTYPE_ALL,
'count' => 2,
'expectedusers' => [
'a',
'c',
],
],
'ALL: Filter on a single group, user in multple groups' => (object) [
'loginuser' => 'c',
'groups' => ['groupa'],
'jointype' => filter::JOINTYPE_ALL,
'count' => 2,
'expectedusers' => [
'a',
'c',
],
],
'ALL: Filter on a single group, user in no groups' => (object) [
'loginuser' => 'd',
'groups' => ['groupa'],
'jointype' => filter::JOINTYPE_ALL,
'count' => 0,
'expectedusers' => ['exception'],
],
'ALL: Filter on multiple groups, user in one group (ignore invalid groups)' => (object) [
'loginuser' => 'a',
'groups' => ['groupa', 'groupb'],
'jointype' => filter::JOINTYPE_ALL,
'count' => 2,
'expectedusers' => [
'a',
'c',
],
],
'ALL: Filter on multiple groups, user in multiple groups' => (object) [
'loginuser' => 'c',
'groups' => ['groupa', 'groupb'],
'jointype' => filter::JOINTYPE_ALL,
'count' => 1,
'expectedusers' => [
'c',
],
],
'ALL: Filter on multiple groups or no groups, user in multiple groups (ignore no groups)' => (object) [
'loginuser' => 'c',
'groups' => ['groupa', 'groupb', 'nogroups'],
'jointype' => filter::JOINTYPE_ALL,
'count' => 1,
'expectedusers' => [
'c',
],
],
// Tests for jointype: NONE.
'NONE: No filter, user in one group' => (object) [
'loginuser' => 'a',
'groups' => [],
'jointype' => filter::JOINTYPE_NONE,
'count' => 2,
'expectedusers' => [
'a',
'c',
],
],
'NONE: No filter, user in multiple groups' => (object) [
'loginuser' => 'c',
'groups' => [],
'jointype' => filter::JOINTYPE_NONE,
'count' => 3,
'expectedusers' => [
'a',
'b',
'c',
],
],
'NONE: No filter, user in no groups' => (object) [
'loginuser' => 'd',
'groups' => [],
'jointype' => filter::JOINTYPE_NONE,
'count' => 0,
'expectedusers' => ['exception'],
],
'NONE: Filter on a single group, user in one group' => (object) [
'loginuser' => 'a',
'groups' => ['groupa'],
'jointype' => filter::JOINTYPE_NONE,
'count' => 0,
'expectedusers' => [],
],
'NONE: Filter on a single group, user in multple groups' => (object) [
'loginuser' => 'c',
'groups' => ['groupa'],
'jointype' => filter::JOINTYPE_NONE,
'count' => 1,
'expectedusers' => [
'b',
],
],
'NONE: Filter on a single group, user in no groups' => (object) [
'loginuser' => 'd',
'groups' => ['groupa'],
'jointype' => filter::JOINTYPE_NONE,
'count' => 0,
'expectedusers' => ['exception'],
],
'NONE: Filter on multiple groups, user in one group (ignore invalid groups)' => (object) [
'loginuser' => 'a',
'groups' => ['groupa', 'groupb'],
'jointype' => filter::JOINTYPE_NONE,
'count' => 0,
'expectedusers' => [],
],
'NONE: Filter on multiple groups, user in multiple groups' => (object) [
'loginuser' => 'c',
'groups' => ['groupa', 'groupb'],
'jointype' => filter::JOINTYPE_NONE,
'count' => 0,
'expectedusers' => [],
],
'NONE: Filter on multiple groups or no groups, user in multiple groups (ignore no groups)' => (object) [
'loginuser' => 'c',
'groups' => ['groupa', 'groupb', 'nogroups'],
'jointype' => filter::JOINTYPE_NONE,
'count' => 0,
'expectedusers' => [],
],
],
],
];
$finaltests = [];
foreach ($tests as $testname => $testdata) {
foreach ($testdata->expect as $expectname => $expectdata) {
$finaltests["{$testname} => {$expectname}"] = [
'users' => $testdata->users,
'groupsavailable' => $testdata->groupsavailable,
'filtergroups' => $expectdata->groups,
'jointype' => $expectdata->jointype,
'count' => $expectdata->count,
'expectedusers' => $expectdata->expectedusers,
'loginusername' => $expectdata->loginuser,
];
}
}
return $finaltests;
}
/**
* Ensure that the last access filter works as expected with the provided test cases.
*