Skip to content

Commit

Permalink
MDL-68093 groups: Add visibility and participation settings
Browse files Browse the repository at this point in the history
These new settings are designed to enchance user privacy surrounding
groups. They allow groups to be configured so that users outside the
group cannot see the group, so that users in the group cannot see each
other, or so that users cannot see the group at all, even if they are in
it. This avoids issues where a group may be assigned based on sensitive
personal information (such as a person requiring special arrangements
due to a disability).

By default, groups are visible to all and available for participation in
activities, which maintains the current behaviour.

For performance, a new cache has been added to track the number of
groups on a course that are not visible to non-members. This allows us
to revert to the existing behaviour if the new features are not being
used at all on a course, and only apply the new visibility conditions if
they are.

Users who have the moodle/course:viewhiddengroups capability should be
concious of exposing hidden groups when showing their screen to other
users. The "Switch role to..." feature can be used to show a course page
on screen without exposing private availability conditions, for example.

The changes cover several specific areas:
* grouplib functions, which most code should use to get lists of groups
  and members (this includes the participants page).
* Activities supporting group overrides will not allow overrides for
  groups that are hidden from all users.
* Activities supporting separate/visible groups modes will only allow
  groups with the new "participation" flag enabled to be selected.
* Group messaging will be disabled for groups where members cannot see
  each other, or cannot see the group at all.
  • Loading branch information
marxjohnson committed Mar 14, 2023
1 parent 5e1df25 commit 958da5b
Show file tree
Hide file tree
Showing 30 changed files with 1,470 additions and 45 deletions.
2 changes: 1 addition & 1 deletion backup/moodle2/backup_stepslib.php
Expand Up @@ -1341,7 +1341,7 @@ protected function define_structure() {

$group = new backup_nested_element('group', array('id'), array(
'name', 'idnumber', 'description', 'descriptionformat', 'enrolmentkey',
'picture', 'timecreated', 'timemodified'));
'picture', 'visibility', 'participation', 'timecreated', 'timemodified'));

$members = new backup_nested_element('group_members');

Expand Down
1 change: 1 addition & 0 deletions group/autogroup.php
Expand Up @@ -234,6 +234,7 @@
$newgroup->courseid = $data->courseid;
$newgroup->name = $group['name'];
$newgroup->enablemessaging = $data->enablemessaging ?? 0;
$newgroup->visibility = GROUPS_VISIBILITY_ALL;
$groupid = groups_create_group($newgroup);
$createdgroups[] = $groupid;
foreach($group['members'] as $user) {
Expand Down
172 changes: 172 additions & 0 deletions group/classes/visibility.php
@@ -0,0 +1,172 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.

/**
* Group visibility methods
*
* @package core_group
* @copyright 2022 onwards Catalyst IT EU {@link https://catalyst-eu.net}
* @author Mark Johnson <mark.johnson@catalyst-eu.net>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/

namespace core_group;

/**
* Group visibility methods.
*/
class visibility {

/**
* Store the number groups with visibility other than ALL on the course.
*
* @param int $courseid Course ID to update the cache for.
* @param \cache|null $cache Existing cache instance. If null, once will be created.
* @return void
* @throws \dml_exception
*/
public static function update_hiddengroups_cache(int $courseid, ?\cache $cache = null): void {
global $DB;
if (!$cache) {
$cache = \cache::make('core', 'coursehiddengroups');
}
$hiddengroups = $DB->count_records_select('groups', 'courseid = ? AND visibility != ?',
[$courseid, GROUPS_VISIBILITY_ALL]);
$cache->set($courseid, $hiddengroups);
}

/**
* Return whether a course currently had hidden groups.
*
* This can be used as a shortcut to decide whether visibility restrictions need to be applied. If this returns false,
* we may be able to use cached data, or do a much simpler query.
*
* @param int $courseid
* @return bool
* @throws \coding_exception
* @throws \dml_exception
*/
public static function course_has_hidden_groups(int $courseid): bool {
$cache = \cache::make('core', 'coursehiddengroups');
$hiddengroups = $cache->get($courseid);
if ($hiddengroups === false) {
self::update_hiddengroups_cache($courseid, $cache);
$cache->get($courseid);
}
return $hiddengroups > 0;
}

/**
* Can the current user view all the groups on the course?
*
* Returns true if there are no groups on the course with visibility != ALL,
* or if the user has viewhiddengroups.
*
* This is useful for deciding whether we need to perform additional visibility checkes
* such as the sql_* methods of this class.
*
* @param int $courseid
* @return bool
*/
public static function can_view_all_groups(int $courseid): bool {
$viewhidden = has_capability('moodle/course:viewhiddengroups', \context_course::instance($courseid));
$hashidden = self::course_has_hidden_groups($courseid);
return $viewhidden || !$hashidden;
}

/**
* Return SQL conditions for determining whether a user can see a group and its memberships.
*
* @param int $userid
* @param string $groupsalias The SQL alias being used for the groups table.
* @param string $groupsmembersalias The SQL alias being used for the groups_members table.
* @return array [$where, $params]
*/
public static function sql_group_visibility_where(int $userid,
string $groupsalias = 'g', string $groupsmembersalias = 'gm'): array {
global $USER;
// Apply visibility restrictions.
// Everyone can see who is in groups with ALL visibility.
$where = "({$groupsalias}.visibility = :all";
$params['all'] = GROUPS_VISIBILITY_ALL;
if ($userid == $USER->id) {
// If the user is looking at their own groups, they can see those with MEMBERS or OWN visibility.
$where .= " OR {$groupsalias}.visibility IN (:members, :own)";
$params['members'] = GROUPS_VISIBILITY_MEMBERS;
$params['own'] = GROUPS_VISIBILITY_OWN;
} else {
list($memberssql, $membersparams) = self::sql_members_visibility_condition($groupsalias, $groupsmembersalias);
// If someone else's groups, they can see those with MEMBERS visibilty, only if they are a member too.
$where .= " OR ($memberssql)";
$params = array_merge($params, $membersparams);
}
$where .= ")";
return [$where, $params];
}

/**
* Return SQL conditions for determining whether a user can see a group's members.
*
* @param string $groupsalias The SQL alias being used for the groups table.
* @param string $groupsmembersalias The SQL alias being used for the groups_members table.
* @param string $useralias The SQL alias being used for the user table.
* @return array [$where, $params]
*/
public static function sql_member_visibility_where(string $groupsalias = 'g',
string $groupsmembersalias = 'gm', string $useralias = 'u'): array {
global $USER;

list($memberssql, $membersparams) = self::sql_members_visibility_condition($groupsalias, $groupsmembersalias);

$where = " AND (
{$groupsalias}.visibility = :all
OR ($memberssql)
OR ({$groupsalias}.visibility = :own AND {$useralias}.id = :currentuser2)
)";
$params = [
'all' => GROUPS_VISIBILITY_ALL,
'own' => GROUPS_VISIBILITY_OWN,
'currentuser2' => $USER->id,
];
$params = array_merge($params, $membersparams);
return [$where, $params];
}

/**
* Return a condition to check if a user can view a group because it has MEMBERS visibility and they are a member.
*
* @param string $groupsalias The SQL alias being used for the groups table.
* @param string $groupsmembersalias The SQL alias being used for the groups_members table.
* @return array [$sql, $params]
*/
protected static function sql_members_visibility_condition(string $groupsalias = 'g',
string $groupsmembersalias = 'gm'): array {
global $USER;
$sql = "{$groupsalias}.visibility = :members
AND (
SELECT gm2.id
FROM {groups_members} gm2
WHERE gm2.groupid = {$groupsmembersalias}.groupid
AND gm2.userid = :currentuser
) IS NOT NULL";
$params = [
'members' => GROUPS_VISIBILITY_MEMBERS,
'currentuser' => $USER->id
];

return [$sql, $params];
}
}
86 changes: 77 additions & 9 deletions group/externallib.php
Expand Up @@ -22,6 +22,7 @@
use core_external\external_value;
use core_external\external_warnings;
use core_external\util;
use core_group\visibility;

/**
* Group external functions
Expand All @@ -34,6 +35,26 @@
*/
class core_group_external extends external_api {


/**
* Validate visibility.
*
* @param int $visibility Visibility string, must one of the visibility class constants.
* @throws invalid_parameter_exception if visibility is not an allowed value.
*/
protected static function validate_visibility(int $visibility): void {
$allowed = [
GROUPS_VISIBILITY_ALL,
GROUPS_VISIBILITY_MEMBERS,
GROUPS_VISIBILITY_OWN,
GROUPS_VISIBILITY_NONE,
];
if (!array_key_exists($visibility, $allowed)) {
throw new invalid_parameter_exception('Invalid group visibility provided. Must be one of '
. join(',', $allowed));
}
}

/**
* Returns description of method parameters
*
Expand All @@ -51,7 +72,14 @@ public static function create_groups_parameters() {
'description' => new external_value(PARAM_RAW, 'group description text'),
'descriptionformat' => new external_format_value('description', VALUE_DEFAULT),
'enrolmentkey' => new external_value(PARAM_RAW, 'group enrol secret phrase', VALUE_OPTIONAL),
'idnumber' => new external_value(PARAM_RAW, 'id number', VALUE_OPTIONAL)
'idnumber' => new external_value(PARAM_RAW, 'id number', VALUE_OPTIONAL),
'visibility' => new external_value(PARAM_INT,
'group visibility mode. 0 = Visible to all. 1 = Visible to members. '
. '2 = See own membership. 3 = Membership is hidden. default: 0',
VALUE_DEFAULT, 0),
'participation' => new external_value(PARAM_BOOL,
'activity participation enabled? Only for "all" and "members" visibility. Default true.',
VALUE_DEFAULT, true),
)
), 'List of group object. A group has a courseid, a name, a description and an enrolment key.'
)
Expand Down Expand Up @@ -101,6 +129,9 @@ public static function create_groups($groups) {
// Validate format.
$group->descriptionformat = util::validate_format($group->descriptionformat);

// Validate visibility.
self::validate_visibility($group->visibility);

// finally create the group
$group->id = groups_create_group($group, false);
if (!isset($group->enrolmentkey)) {
Expand Down Expand Up @@ -134,7 +165,11 @@ public static function create_groups_returns() {
'description' => new external_value(PARAM_RAW, 'group description text'),
'descriptionformat' => new external_format_value('description'),
'enrolmentkey' => new external_value(PARAM_RAW, 'group enrol secret phrase'),
'idnumber' => new external_value(PARAM_RAW, 'id number')
'idnumber' => new external_value(PARAM_RAW, 'id number'),
'visibility' => new external_value(PARAM_INT,
'group visibility mode. 0 = Visible to all. 1 = Visible to members. 2 = See own membership. '
. '3 = Membership is hidden.'),
'participation' => new external_value(PARAM_BOOL, 'participation mode'),
)
), 'List of group object. A group has an id, a courseid, a name, a description and an enrolment key.'
);
Expand Down Expand Up @@ -168,7 +203,8 @@ public static function get_groups($groupids) {
$groups = array();
foreach ($params['groupids'] as $groupid) {
// validate params
$group = groups_get_group($groupid, 'id, courseid, name, idnumber, description, descriptionformat, enrolmentkey', MUST_EXIST);
$group = groups_get_group($groupid, 'id, courseid, name, idnumber, description, descriptionformat, enrolmentkey, '
. 'visibility, participation', MUST_EXIST);

// now security checks
$context = context_course::instance($group->courseid, IGNORE_MISSING);
Expand Down Expand Up @@ -208,7 +244,11 @@ public static function get_groups_returns() {
'description' => new external_value(PARAM_RAW, 'group description text'),
'descriptionformat' => new external_format_value('description'),
'enrolmentkey' => new external_value(PARAM_RAW, 'group enrol secret phrase'),
'idnumber' => new external_value(PARAM_RAW, 'id number')
'idnumber' => new external_value(PARAM_RAW, 'id number'),
'visibility' => new external_value(PARAM_INT,
'group visibility mode. 0 = Visible to all. 1 = Visible to members. 2 = See own membership. '
. '3 = Membership is hidden.'),
'participation' => new external_value(PARAM_BOOL, 'participation mode'),
)
)
);
Expand Down Expand Up @@ -251,7 +291,8 @@ public static function get_course_groups($courseid) {
require_capability('moodle/course:managegroups', $context);

$gs = groups_get_all_groups($params['courseid'], 0, 0,
'g.id, g.courseid, g.name, g.idnumber, g.description, g.descriptionformat, g.enrolmentkey');
'g.id, g.courseid, g.name, g.idnumber, g.description, g.descriptionformat, g.enrolmentkey, '
. 'g.visibility, g.participation');

$groups = array();
foreach ($gs as $group) {
Expand Down Expand Up @@ -280,7 +321,11 @@ public static function get_course_groups_returns() {
'description' => new external_value(PARAM_RAW, 'group description text'),
'descriptionformat' => new external_format_value('description'),
'enrolmentkey' => new external_value(PARAM_RAW, 'group enrol secret phrase'),
'idnumber' => new external_value(PARAM_RAW, 'id number')
'idnumber' => new external_value(PARAM_RAW, 'id number'),
'visibility' => new external_value(PARAM_INT,
'group visibility mode. 0 = Visible to all. 1 = Visible to members. 2 = See own membership. '
. '3 = Membership is hidden.'),
'participation' => new external_value(PARAM_BOOL, 'participation mode'),
)
)
);
Expand Down Expand Up @@ -1499,7 +1544,12 @@ public static function update_groups_parameters() {
'description' => new external_value(PARAM_RAW, 'group description text', VALUE_OPTIONAL),
'descriptionformat' => new external_format_value('description', VALUE_DEFAULT),
'enrolmentkey' => new external_value(PARAM_RAW, 'group enrol secret phrase', VALUE_OPTIONAL),
'idnumber' => new external_value(PARAM_RAW, 'id number', VALUE_OPTIONAL)
'idnumber' => new external_value(PARAM_RAW, 'id number', VALUE_OPTIONAL),
'visibility' => new external_value(PARAM_TEXT,
'group visibility mode. 0 = Visible to all. 1 = Visible to members. '
. '2 = See own membership. 3 = Membership is hidden.', VALUE_OPTIONAL),
'participation' => new external_value(PARAM_BOOL,
'activity participation enabled? Only for "all" and "members" visibility', VALUE_OPTIONAL),
)
), 'List of group objects. A group is found by the id, then all other details provided will be updated.'
)
Expand All @@ -1523,13 +1573,13 @@ public static function update_groups($groups) {
$transaction = $DB->start_delegated_transaction();

foreach ($params['groups'] as $group) {
$group = (object)$group;
$group = (object) $group;

if (trim($group->name) == '') {
throw new invalid_parameter_exception('Invalid group name');
}

if (! $currentgroup = $DB->get_record('groups', array('id' => $group->id))) {
if (!$currentgroup = $DB->get_record('groups', array('id' => $group->id))) {
throw new invalid_parameter_exception("Group $group->id does not exist");
}

Expand All @@ -1539,6 +1589,24 @@ public static function update_groups($groups) {
throw new invalid_parameter_exception('A different group with the same name already exists in the course');
}

if (isset($group->visibility) || isset($group->participation)) {
$hasmembers = $DB->record_exists('groups_members', ['groupid' => $group->id]);
if (isset($group->visibility)) {
// Validate visibility.
self::validate_visibility($group->visibility);
if ($hasmembers && $group->visibility != $currentgroup->visibility) {
throw new invalid_parameter_exception(
'The visibility of this group cannot be changed as it currently has members.');
}
} else {
$group->visibility = $currentgroup->visibility;
}
if (isset($group->participation) && $hasmembers && $group->participation != $currentgroup->participation) {
throw new invalid_parameter_exception(
'The participation mode of this group cannot be changed as it currently has members.');
}
}

$group->courseid = $currentgroup->courseid;

// Now security checks.
Expand Down

0 comments on commit 958da5b

Please sign in to comment.