Course completed enrolment
enrol_coursecompleted
Course completed enrolment plugin. When a learner completes a configured "source" course, the plugin automatically enrols them into the course where the instance is configured (the "destination" course). It supports chained learning paths, future-dated enrolments via adhoc tasks, group propagation, an optional self-unenrol of the source course after the destination enrolment, bulk edit/delete actions for admins, and an admin manage page to bulk-enrol users who completed the source course in the past.
The plugin is well-structured and follows Moodle conventions throughout. Database access is uniformly via the $DB API with parameterised queries, capability checks gate every privileged path (manage.php, bulk operations, action icons, instance management), the privacy provider is implemented (null provider — the plugin stores no personal data of its own), uninstall cleans up role assignments and queued adhoc tasks, and the welcome message renders through the core send_course_welcome_message_to_user API which applies format_text sanitisation. No SQL injection, XSS, CSRF, file inclusion, command execution, or capability bypass was identified. The only findings are minor code-quality issues: manage.php orders capability checks before require_login() (sequence is unconventional but not exploitable because guest/anonymous users do not hold the required capabilities), uses PARAM_RAW for an action parameter that is only ever string-compared, passes fullname() output to confirm() without explicit escaping (mitigated by PARAM_NOTAGS on stored user names), the Mustache template uses an unescaped placeholder inside an HTML title attribute, and a language string for a cache that is never declared remains in the lang file. None of these represent a security risk in practice.
Overview
enrol_coursecompleted is a Moodle enrolment plugin that automatically enrols a user into a destination course when they complete a configured prerequisite course. The plugin is mature (the changelog goes back to 3.8) and is published with active CI, mutation testing, and code coverage.
Security posture
The plugin uses appropriate Moodle APIs throughout:
- Database access is exclusively through
$DBwith placeholders. There are no rawmysqli_*/PDOcalls and no string-concatenated SQL with user data. The two SQLLIKEpatterns inclasses/hook_listener.phpinterpolatecourse->idandenrol instance id— both internal integer identifiers, not user input. - Capabilities are checked at every state-changing entry point:
manage.phprequiresenrol/coursecompleted:enrolpast,enrol/coursecompleted:unenrol, orenrol/manual:enrol;bulkdelete::processandbulkedit::processrequireenrol/coursecompleted:unenrolandenrol/coursecompleted:managerespectively;can_add_instance,can_delete_instance,can_hide_show_instance,allow_unenrol, andallow_manageall checkenrol/coursecompleted:manage. - CSRF is enforced via
require_sesskey()in the state-changing branch ofmanage.php, and the bulk forms inherit sesskey handling from the parentmoodleformclasses. - Output sanitisation routes the formatted course name through
\core\formatting::format_string()before it reaches Mustache or language-string placeholders. The custom welcome message is delivered via the coresend_course_welcome_message_to_user()helper which appliesformat_text(..., FORMAT_MOODLE). - Lifecycle hygiene:
db/install.phpenables the plugin on install;db/uninstall.phpremoves role assignments, deletes plugin enrol instances, and clears related adhoc tasks;classes/hook_listener.phpcleans up future-dated adhoc tasks when the source course is deleted or the instance is disabled. - Privacy is declared via
\core_privacy\local\metadata\null_provider— accurate, since the plugin only writes to standard core enrolment tables and stores no personal data of its own.
Findings
No security vulnerabilities were identified. A handful of low/info-level code-quality observations are included in findings, the most notable being the unconventional ordering of capability checks before require_login() in manage.php and the use of an unescaped Mustache placeholder inside an HTML title attribute. Neither is exploitable in practice.
Third-party code
The plugin ships no third-party libraries — there is no thirdpartylibs.xml, no vendor directory, and no bundled JavaScript. All code is the plugin author's, and the only templating dependency is core Moodle's Mustache renderer.
Findings
In manage.php, the calls to has_capability() and require_capability() occur before require_login($course). The standard Moodle pattern is to call require_login() first, then perform capability checks.
In practice this is not directly exploitable: a guest or unauthenticated user does not hold enrol/coursecompleted:enrolpast, enrol/coursecompleted:unenrol, or enrol/manual:enrol, so require_capability() will throw required_capability_exception and the page exits with the standard "no permissions" error. However, the inverted ordering removes a defence-in-depth layer (forced login, MUST_ACCESS_KEY checks, session re-validation, course-availability and login-as handling) that require_login() provides before any course-specific access decision is made.
Additionally, the $context variable is assigned only inside the if ($instance = ...) block guarded by MUST_EXIST — if $DB->get_record(..., MUST_EXIST) ever returned false instead of throwing (it won't, but the if is dead code that suggests otherwise), $context would be undefined when the capability check runs.
Low risk. The capability checks effectively gate access — guest and unauthenticated users will be rejected with required_capability_exception. The finding is a defence-in-depth and code-conventions concern, not an exploitable bypass. Authenticated callers without the required capability are still rejected. There is no known scenario in which a user reaches the privileged action handler without holding one of the required capabilities.
manage.php is the back-office page used by managers/editing teachers to retroactively enrol users who completed the source course in the past. It is reachable directly by URL when the user has the right capability via the t/enrolusers icon rendered in enrol_coursecompleted_plugin::get_action_icons().
if ($instance = $DB->get_record('enrol', ['id' => $enrolid, 'enrol' => 'coursecompleted'], '*', MUST_EXIST)) {
$course = get_course($instance->courseid);
$context = \context_course::instance($course->id, MUST_EXIST);
}
$canenrol = has_capability('enrol/coursecompleted:enrolpast', $context);
$canunenrol = has_capability('enrol/coursecompleted:unenrol', $context);
if (!$canenrol && !$canunenrol) {
// No need to invent new error strings here...
require_capability('enrol/manual:enrol', $context);
}
require_login($course);
Move require_login($course) ahead of the capability checks and drop the redundant if ($instance = ...) wrapper since MUST_EXIST already throws on missing records:
$instance = $DB->get_record('enrol', ['id' => $enrolid, 'enrol' => 'coursecompleted'], '*', MUST_EXIST);
$course = get_course($instance->courseid);
$context = \context_course::instance($course->id, MUST_EXIST);
require_login($course);
$canenrol = has_capability('enrol/coursecompleted:enrolpast', $context);
$canunenrol = has_capability('enrol/coursecompleted:unenrol', $context);
if (!$canenrol && !$canunenrol) {
require_capability('enrol/manual:enrol', $context);
}
manage.php reads the action request parameter with PARAM_RAW, which performs no cleaning. The value is only used in a single literal string comparison ($action === 'enrol'), so it is not directly dangerous, but selecting a permissive type for a value that should be an alphabetic action name is a coding-standards issue and may become a problem if the value is later logged, displayed, or re-used in another context.
Low risk. Because the value is only used in an equality comparison against a literal string, the lack of cleaning has no exploitable effect today. The finding is purely a hygiene / convention issue.
The action parameter selects between the confirmation page and the actual enrol operation. It is only ever compared to the literal string 'enrol', and the state-changing branch is gated by require_sesskey() and the earlier capability checks.
$action = optional_param('action', '', PARAM_RAW);
Use PARAM_ALPHA to constrain the value to alphabetic characters — sufficient for the only accepted value ('enrol'):
$action = optional_param('action', '', PARAM_ALPHA);
In manage.php, the list of candidate users is built with fullname($user) and joined into a comma-separated string that is then passed as the message argument to $OUTPUT->confirm(). $OUTPUT->confirm() wraps the message in a <p> via html_writer::tag('p', $message) without escaping its contents (html_writer::tag() does not escape body content), so any HTML in the message is rendered as HTML.
In practice this is mitigated because Moodle stores firstname/lastname with PARAM_NOTAGS, stripping HTML tags at input. Therefore a <script> payload cannot survive the user profile save path. However, the plugin should not rely on that input filter as the only line of defence — names from external auth sources (LDAP/SAML/oauth2) or future-modified core behaviour could carry untrusted characters, and the convention in Moodle output code is to apply s() or format_string() to any user-supplied text before mixing it into HTML.
Low risk. Moodle's PARAM_NOTAGS definition on firstname, lastname, and surname (/moodle/public/lib/classes/user.php:738-740) strips all HTML tags before they reach the database, so a typical user cannot inject <script> via their profile. The finding is defence-in-depth — the output path itself does not escape, and relying on a single upstream sanitiser is fragile. No realistic exploit path was identified given current core behaviour.
The page is reachable by users holding enrol/coursecompleted:enrolpast, enrol/coursecompleted:unenrol, or enrol/manual:enrol on the destination course (typically teachers/managers). It enumerates users who have a course_completions record with timecompleted > 0 for the configured source course.
foreach ($candidates as $candidate) {
if (!isset($current[$candidate])) {
$user = \core_user::get_user($candidate);
if (!empty($user) && !$user->deleted) {
$allusers[$candidate] = fullname($user);
}
}
}
if ($allusers !== []) {
$link = new \moodle_url($PAGE->url, ['enrolid' => $enrolid, 'action' => 'enrol', 'sesskey' => sesskey()]);
echo $OUTPUT->confirm(
implode(', ', $allusers),
new \single_button($link, get_string('manual:enrol', 'enrol_manual')),
$cancelurl
);
}
Escape each name before joining:
$allusers[$candidate] = s(fullname($user));
or wrap the full list at the point of output:
echo $OUTPUT->confirm(
s(implode(', ', $allusers)),
new \single_button($link, get_string('manual:enrol', 'enrol_manual')),
$cancelurl
);
templates/learnpath.mustache uses the triple-brace {{{title}}} form inside the title attribute of an anchor. Triple-brace disables Mustache's automatic HTML escaping. The value supplied for title is the result of \core\formatting::format_string($course->fullname, ...) in classes/plugin.php, which strips tags and substitutes </> with </>, so in current behaviour the attribute is safe.
However, the triple-brace form is unnecessary here — the corresponding title attribute in line 45 uses the double-brace form ({{coursetitle}}) for what is the same kind of data, so the inconsistency is also a hint that the triple-brace was unintentional. Removing the extra brace yields the same visible output and removes the dependency on format_string's exact escape behaviour.
Low risk. Currently safe — format_string() removes/escapes HTML before the data reaches the template. No XSS path exists at present. The finding is a robustness/convention concern in case format_string behaviour changes or the data source is later swapped.
The template renders the learning-path navigation on the destination course's enrol page. The title value originates from format_string($course->fullname) which already neutralises HTML special characters before the value reaches Mustache.
<a href="{{href}}" title="{{{title}}}">
Use the standard escaped form:
<a href="{{href}}" title="{{title}}">
In enrol_coursecompleted_plugin::enrol_user(), the "try unenrol from completed course" branch iterates over the user's enrolments and reassigns the loop variable $instance from the outer method parameter (the coursecompleted instance) to whatever enrolment instance is being processed for the source course.
This is functionally harmless today because the loop is the last statement in the method, but it makes the code harder to reason about and would silently introduce a bug if any code is added after the loop that assumes $instance still refers to the coursecompleted instance.
Informational. No behavioural impact today. Flagged for maintainability — future edits below the loop could be subtly broken by the variable reuse.
This block implements the optional "unenrol the user from the completed course" feature (controlled by customint5). It runs as the final step in enrol_user().
foreach ($enrols as $enrol) {
$plugin = enrol_get_plugin($enrol->enrolmentinstance->enrol);
$instance = $DB->get_record('enrol', ['id' => $enrol->enrolid], '*', MUST_EXIST);
if ($plugin->allow_unenrol_user($instance, $enrol)) {
$plugin->unenrol_user($instance, $userid);
}
}
Use a distinct loop-local name:
foreach ($enrols as $enrol) {
$plugin = enrol_get_plugin($enrol->enrolmentinstance->enrol);
$sourceinstance = $DB->get_record('enrol', ['id' => $enrol->enrolid], '*', MUST_EXIST);
if ($plugin->allow_unenrol_user($sourceinstance, $enrol)) {
$plugin->unenrol_user($sourceinstance, $userid);
}
}
The language file declares $string['cachedef_compcourses'] = 'Enrolment on course completion cache';, but the plugin contains no db/caches.php, no \cache::make('enrol_coursecompleted', 'compcourses', ...) call, and no reference to a cache definition with that name. The string is therefore dead — likely a leftover from a feature that was removed or never finished.
Informational. No security or functional impact. Indicates minor technical debt.
Moodle expects a cachedef_<name> string to exist whenever a cache definition is registered in db/caches.php. Without the matching db/caches.php, the string is unreachable.
$string['cachedef_compcourses'] = 'Enrolment on course completion cache';
Either add a db/caches.php defining a cache named compcourses and use it from the plugin, or remove the unused string from the language file.
lib.php contains only the GPL header and a docblock — no function or constant declarations. Moodle does not require an enrolment plugin to ship lib.php, so keeping an empty file is harmless but unnecessary; it can be deleted unless something in the wider environment (e.g., a code-scanner expecting the file) requires its presence.
Informational. No impact.
Plugin authors sometimes keep an empty lib.php as a placeholder. There is no requirement to ship one for an enrol_* plugin.
Delete lib.php if no consumer relies on its existence, or add a single explanatory comment in the file to make the intent clear.
xmldb_enrol_coursecompleted_install() modifies $CFG->enrol_plugins_enabled to add coursecompleted so the plugin is enabled immediately after install. This is a common pattern for enrolment plugins (enrol_self, enrol_manual do the same in core) and is convenient, but operators rolling the plugin out across a managed estate may prefer to keep new plugins disabled by default and enable them via an explicit configuration step.
This is not a vulnerability — administrators can disable the plugin from Site administration > Plugins > Enrolments > Manage enrol plugins — but it is worth surfacing.
Informational. A deliberate UX choice. No security impact — administrators retain full control over the enabled list.
The function runs once when the plugin is first installed. After that, the enabled state is managed via the admin UI.
function xmldb_enrol_coursecompleted_install(): bool {
$enabled = enrol_get_plugins(true);
$enabled['coursecompleted'] = true;
set_config('enrol_plugins_enabled', implode(',', array_keys($enabled)));
return true;
}
If preferred, remove the auto-enable so operators must opt in explicitly:
function xmldb_enrol_coursecompleted_install(): bool {
return true;
}
The plugin's GitHub Actions workflows in .github/workflows/ (Moodle CI, infection mutation testing, cron, release) are not part of the deployed plugin and were noted only for context — none of them ship code that runs in a Moodle environment.
The Privacy API implementation uses \core_privacy\local\metadata\null_provider and is accurate: the plugin writes only to standard core enrolment tables (enrol, user_enrolments, role_assignments, task_adhoc) which have their own privacy providers in core. No additional provider work is needed.
db/uninstall.php is comprehensive: it iterates all coursecompleted enrol records and calls delete_instance() (which removes role assignments and user enrolments), calls role_unassign_all(['component' => 'enrol_coursecompleted']) to catch any stragglers, and clears related future-dated adhoc tasks. This is a good example of cleanup hygiene.
The before_course_deleted and after_enrol_instance_status_updated hook listeners use SQL LIKE patterns against the adhoc-task customdata JSON column. The values interpolated into the LIKE pattern are internal integer identifiers (course->id, enrol instance id), not user input, so the pattern is safe — but it is brittle in that it assumes Moodle's $DB layer returns integer fields as strings (which they do via set_custom_data(json_encode($record))). If the underlying storage shape changes, the pattern will silently stop matching.