🕹️PlayerGroup
mod_playergroup
A collaborative course activity (mod_playergroup) that lets students autonomously create, join, leave and manage game-style teams within a Moodle course. Groups are backed by native Moodle groups/groupings and support open, password-protected and invite-only privacy modes, student-to-student invitations, optional join/create grading and completion tracking, teacher activity and composition reports with CSV/Excel export, a Moodle mobile app addon, and full backup/restore plus Privacy API support.
This is a mature, carefully engineered module. Every state-changing web service validates parameters, validates the module context, and enforces an appropriate capability (creategroup, manageinvites, view or manage); object ownership is checked before acting (only an invite's receiverid may accept/reject it, only a group's creatorid may edit it). All database access uses parameterized $DB queries with placeholders and core helpers (get_in_or_equal, get_enrolled_sql) — no string concatenation of user input, no raw DB or HTTP clients, no superglobals, no eval/exec/shell calls, and no direct filesystem access beyond reading bundled plugin JS. Output is consistently escaped via format_string, format_text, Mustache double-brace escaping and Angular interpolation; client-side rendering goes through Templates.render, so there is no DOM-based XSS. Protected-group passwords are stored with password_hash and checked with password_verify. Concurrency is handled with a per-instance lock, and the CSV/Excel export relies on core \core\dataformat which escapes spreadsheet formula injection. The plugin ships a complete Privacy API provider, backup/restore, events, a message provider and an extensive PHPUnit/Behat test suite.
No high or critical issues were found. The only findings are low severity: an information-disclosure inconsistency where group member names are shown to every viewer even though the equivalent invite feature is gated on moodle/course:viewparticipants; direct reads of logstore_standard_log that make the teacher report empty if the standard log store is disabled; a missing form validation() for member bounds; and the ability to create a password-protected group with no password (rendering it unjoinable). All are well constrained, affect the actor's own context or non-sensitive peer names, and do not permit harm to other users' data or privilege escalation.
mod_playergroup is a high-quality Moodle activity module for student-driven group formation. The review covered all PHP, JavaScript and Mustache files, the database/backup/privacy layers, the mobile addon and both CLI seed scripts.
Security posture is strong:
- Access control — every external/AJAX endpoint runs
validate_parameters+validate_context+ arequire_capabilitycheck, and performs object-level ownership checks (invite recipient, group creator) before mutating state. - SQL — exclusively parameterized
$DBqueries and core helpers; no injectable concatenation. - XSS — user data is escaped through
format_string/format_text, Mustache auto-escaping and Angular; raw HTML is only emitted via{{{description}}}afterformat_text/clean_textsanitization. - CSRF — all mutations go through the external-services framework or
moodleform, both of which enforce sesskey; the only custom entry point (export.php) is a read-only GET. - Secrets — group passwords use
password_hash/password_verify. - Robustness — a
\core\lockguards the join/create critical section against races.
Findings (all low):
- Group member names are disclosed to any user with
mod/playergroup:view, inconsistently with the invite picker which is gated onmoodle/course:viewparticipants. - Teacher reports/exports read
logstore_standard_logdirectly and break silently if the standard log store is disabled. - The activity settings form lacks validation of the min/max member fields.
- A protected group can be created with an empty password, making it permanently unjoinable.
No third-party libraries are bundled in the plugin runtime. The CLI seed scripts are correctly gated to development sites.
Findings
The student view and the mobile endpoint build a full member list (every member's fullname) for every group in the activity and expose it to any user who holds mod/playergroup:view. The "View members" button on each group card carries this data unconditionally.
The plugin itself treats the capability moodle/course:viewparticipants as the signal that governs whether a student may browse course-mates by name: the invite picker ($caninviteusers) is gated on it, with an in-code comment stating that a teacher who hides the participant list has decided students should not browse peers by name. The group member display does not honour that same signal.
The result is an inconsistent access-control decision: a student (or a guest, on a guest-accessible course) who is denied moodle/course:viewparticipants can still enumerate the full names and group membership of every student who has joined a group, through the per-group member modal/panel.
Low risk. The data exposed is limited to the full names of enrolled students who have voluntarily joined groups in a collaborative, within-course activity — not grades, contact details or other sensitive data. In a default course students already hold moodle/course:viewparticipants, so nothing extra leaks; the gap only materializes where a teacher has deliberately restricted participant visibility or enabled guest access. There is no write capability and no cross-course reach. The real defect is the inconsistency: the plugin protects ungrouped students' names in the invite picker but not grouped students' names in the member modal. Blast radius is confined to name/membership disclosure within a single course.
view.php bulk-loads all members of all displayed groups ($membersbygroup) and attaches a JSON member list to every card regardless of capability. Separately, at line 88 it computes $caninviteusers = has_capability('moodle/course:viewparticipants', $coursecontext) and uses it (lines 215+) to decide whether to build the invite picker, with a comment explicitly tying participant-list visibility to whether students should see peers by name. get_activity_data.php repeats the same structure for the mobile app. The names themselves are safely escaped, so this is an information-exposure/authorization-consistency issue, not an XSS issue.
- In a course, remove the
moodle/course:viewparticipantscapability from the Student role (or enable guest access, where the guest role likewise lacks it in a hardened setup). - As that student/guest, open the PlayerGroup activity.
- Click View members on any group card (the invite picker is correctly hidden, but this button is not).
- The modal lists the full names of every member of that group — and repeating for each card enumerates the group membership of all grouped students, data the participant-list restriction was meant to withhold.
$groupmembers = [];
foreach ($membersbygroup[$groupid] ?? [] as $member) {
$groupmembers[] = [
'fullname' => fullname($member),
'isleader' => (int) $member->userid === (int) $g->creatorid,
];
}
$groupmembers = \mod_playergroup\local\member_list::order($groupmembers);
$membersjson = json_encode($groupmembers, JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT);
Gate the member list on the same capability the invite feature uses. Only populate members/membersjson for a given card when the viewer may see participants (or when it is the viewer's own group):
$canseemembers = $ismygroup || $caninviteusers; // $caninviteusers = has_capability('moodle/course:viewparticipants', ...)
$membersjson = $canseemembers
? json_encode($groupmembers, JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT)
: json_encode([]);
and only render the "View members" button when $canseemembers is true. Apply the same condition in the mobile endpoint.
$groupmembers = [];
foreach ($membersbygroup[$groupid] ?? [] as $member) {
$groupmembers[] = [
'fullname' => fullname($member),
'isleader' => (int) $member->userid === (int) $g->creatorid,
];
}
$groupmembers = \mod_playergroup\local\member_list::order($groupmembers);
Mirror the web fix: only include members for a group when the caller may see participants (has_capability('moodle/course:viewparticipants', $coursecontext)) or when it is the caller's own group.
<button type="button" class="btn btn-outline-secondary btn-sm pg-btn-viewmembers"
data-action="viewmembers"
data-groupname="{{name}}"
data-members="{{membersjson}}"
aria-label="{{viewmembersarialabel}}">
<i class="fa fa-users me-1" aria-hidden="true"></i>
{{membercount}} / {{maxmembers}}
</button>
Wrap the button in a section flag (e.g. {{#canseemembers}} ... {{/canseemembers}}) set by the controller so the member roster is only offered to viewers permitted to see participants.
The teacher activity-log tab in view.php and the log export controller both read event data straight from the {logstore_standard_log} table. Moodle allows administrators to disable the Standard log store or run an alternative reader (for example the external-database log store), in which case this table is empty even though the plugin's events were recorded through the logging API.
When that happens, the teacher report and its CSV/Excel export silently show no rows, giving the misleading impression that no group activity occurred. The supported approach is to obtain a log reader through the logging API (get_log_manager()->get_readers(\core\log\sql_reader::class)) and query through it, so the report works regardless of which store is active.
Low risk. No security impact: the report is read-only and gated behind mod/playergroup:manage. The consequence is purely functional — on sites that do not use the standard log store, teachers see an empty report and export. This is a code-quality/portability defect rather than a vulnerability.
The plugin triggers well-formed events (group_created, member_joined, etc.) via the events API, which is correct. The reporting side, however, bypasses the logging abstraction and assumes the standard store is the backing table. The queries are parameterized and capped (200 rows / recordset streaming), so there is no injection or memory concern — only a portability/robustness gap.
$logsql = "SELECT l.id, l.timecreated, l.userid, l.eventname
FROM {logstore_standard_log} l
WHERE l.contextid = :contextid
AND l.eventname $inevents
ORDER BY l.timecreated DESC";
Query through the active SQL log reader instead of the table:
$readers = get_log_manager()->get_readers('\\core\\log\\sql_reader');
$reader = reset($readers);
if ($reader) {
$events = $reader->get_events_select(
'contextid = :contextid AND eventname ' . $inevents,
array_merge(['contextid' => $context->id], $ineventsparams),
'timecreated DESC', 0, 200
);
}
This keeps the report working when a non-standard log store is in use.
$logsql = "SELECT l.id, l.timecreated, l.userid, l.eventname,
u.firstname, u.lastname, u.firstnamephonetic,
u.lastnamephonetic, u.middlename, u.alternatename
FROM {logstore_standard_log} l
LEFT JOIN {user} u ON u.id = l.userid
WHERE l.contextid = :contextid
AND l.eventname $inevents
ORDER BY l.timecreated DESC";
Use the log reader API as above and stream its results into \core\dataformat::download_data, rather than reading {logstore_standard_log} directly.
The module settings form declares minmembers and maxmembers as PARAM_INT with defaults but provides no validation() method, so there is no lower bound, upper bound, or cross-field check. A teacher can save maxmembers = 0 (or a negative value), or set minmembers greater than maxmembers.
With maxmembers = 0, the join logic ($membercount >= (int) $playergroup->maxmembers) treats every group as already full, so no student can join any group and the UI marks all groups full. A minmembers > maxmembers configuration is accepted silently and simply never becomes satisfiable. These are self-inflicted misconfigurations, but core forms normally guard against them.
Low risk. This affects only the configuring teacher's own activity and cannot harm other users or data — it is a usability/robustness gap, not a security issue. Impact is a non-functional activity until the teacher corrects the setting.
maxmembers is read as an integer wherever capacity is checked (join_group, accept_invite, send_invite, the card rendering), so a zero/negative value propagates into the >= capacity comparisons and the isfull flag. Only a teacher/manager with mod/playergroup:addinstance can reach the form.
$mform->addElement('text', 'minmembers', get_string('minmembers', 'mod_playergroup'), ['size' => '5']);
$mform->setType('minmembers', PARAM_INT);
$mform->setDefault('minmembers', 2);
$mform->addElement('text', 'maxmembers', get_string('maxmembers', 'mod_playergroup'), ['size' => '5']);
$mform->setType('maxmembers', PARAM_INT);
$mform->setDefault('maxmembers', 5);
Add a validation() override to enforce sane bounds:
public function validation($data, $files) {
$errors = parent::validation($data, $files);
if ((int) $data['maxmembers'] < 1) {
$errors['maxmembers'] = get_string('err_maxmembersmin', 'mod_playergroup');
}
if ((int) $data['minmembers'] > (int) $data['maxmembers']) {
$errors['minmembers'] = get_string('err_minovermax', 'mod_playergroup');
}
return $errors;
}
When creating a group, selecting privacy level 1 (protected) does not require a password. create_group only hashes a password when the supplied value is non-empty, otherwise it stores an empty string:
If a student picks "Protected" and leaves the password blank, the group is saved with privacy = 1 and an empty password. join_group then rejects every attempt because empty($meta->password) is true, so no other student can ever join. The creator (already a member) is left with a group nobody else can enter.
The client-side create modal does not mark the password field required and creategroup.js does not validate it, so nothing on either side prevents the state. The same gap exists when editing an open group into protected mode with a blank password.
Low risk. No security consequence and no impact on other users beyond the creator's own group being unusable; the student can simply leave (triggering auto-delete of the empty group, if enabled) and recreate it correctly. This is a data-integrity/UX correctness gap rather than a vulnerability.
join_group guards protected groups with if (empty($meta->password) || !password_verify($params['password'], $meta->password)). The empty($meta->password) branch exists to prevent joining a passwordless protected group, but the create/edit paths never prevent such a group from being created in the first place, so the two halves combine into a permanently unjoinable group.
$privacylevel = (int) $params['privacy'];
$hashedpassword = '';
if ($privacylevel === 1 && $params['password'] !== '') {
$hashedpassword = password_hash($params['password'], PASSWORD_DEFAULT);
}
Reject a protected group with no password rather than storing an empty hash:
if ($privacylevel === 1 && $params['password'] === '') {
throw new \moodle_exception('passwordrequired', 'mod_playergroup');
}
Apply the equivalent check in edit_group when switching to protected mode with no existing and no new password.
var name = modal.getRoot().find('#pg-groupname').val();
var desc = modal.getRoot().find('#pg-groupdesc').val();
var badge = modal.getRoot().find('#pg-groupbadge').val();
var privacy = parseInt(modal.getRoot().find('#pg-groupprivacy').val(), 10);
var password = modal.getRoot().find('#pg-grouppassword').val();
if (!name.trim()) {
modal.getRoot().find('#pg-groupname').addClass('is-invalid');
return;
}
When privacy === 1, require a non-empty password before calling the web service (and surface an inline validation message), mirroring the server-side check:
if (privacy === 1 && !password.trim()) {
modal.getRoot().find('#pg-grouppassword').addClass('is-invalid');
return;
}
Overall engineering quality is high. The plugin demonstrates consistent, defence-in-depth use of Moodle APIs: validate_parameters + validate_context + require_capability on every external function, ownership checks before mutation, a \core\lock factory to serialize the join/create critical section against race conditions, and password_hash/password_verify for protected-group passwords.
CSV/Excel export is safe from formula injection. The export controllers stream through \core\dataformat::download_data, and core's spout_base::write_record runs every cell through \core\dataformat::escape_spreadsheet_formula, which prefixes values beginning with =, +, - or @ per OWASP guidance. No additional sanitization is required in the plugin.
No third-party libraries are bundled in the plugin runtime, so a thirdpartylibs.xml is not required. The JavaScript under docs/assets/js/ belongs to the project's GitHub Pages documentation site and is not installed or executed by Moodle. The amd/build/*.min.js files are compiled outputs of the plugin's own amd/src sources.
The CLI seed scripts are responsibly gated. cli/seed.php and cli/seed_pt_br.php set CLI_SCRIPT, require an explicit --password, set $CFG->noemailever, and refuse to run unless $CFG->wwwroot matches a development pattern (localhost, 127.0.0.1, .local, .test) or --force is passed. The hardcoded SEED_GROUP_PASSWORD is demo data for a seeded group on development sites only, not a production credential.
Privacy, backup and MOODLE_INTERNAL hygiene are correct. The Privacy API provider implements metadata, context/user discovery, export and all deletion paths with parameterized SQL; backup/restore remaps all foreign keys and files; and the defined('MOODLE_INTERNAL') || die(); guard is present exactly on files with file-scope side effects (db/*, version.php, mod_form.php, backup task classes) and correctly absent from autoloaded single-class files and the function-only lib.php.
Public cross-plugin API. classes/api/group_info.php is a read-only data accessor intended for other plugins (e.g. block_playerhud). It performs no capability checks by design, delegating access control to its callers; this is appropriate for a library helper but worth keeping in mind as consuming plugins are reviewed.