MDL Shield

Automatic User Lifecycle Management

tool_userautodelete

Print Report
Plugin Information

An admin tool that automatically notifies, suspends, anonymizes and deletes users through configurable multi-step workflows. Users are selected by composable filter sub-plugins (last access, auth method, cohort, role, enrolment, profile field, confirmation/suspension state, date/delay) and acted upon by action sub-plugins (mail, suspend/unsuspend, anonymize, delete, badge purge, cohort membership, profile-field set). Workflow execution runs via scheduled tasks and is managed entirely from the site administration area.

Version:2026100300
Release:2.5.0
Reviewed for:5.2
Privacy API
Unit Tests
Behat Tests
Reviewed:2026-10-03
227 files·32,038 lines
Grade Justification

The plugin is a destructive, high-impact tool (it deletes and anonymizes users), yet its entire control surface is correctly locked down. Every web entry point (workflows.php, workflow.php, dryrun.php, log.php, and all manage*.php handlers) enforces site-administrator access through require_admin(), admin_externalpage_setup(), or a custom admin_hidden_externalpage_setup() helper that itself calls require_admin(). Every state-changing GET action calls require_sesskey(), and confirmation/edit operations use moodleform/dynamic_form, which enforce the sesskey at construction time — verified against core formslib.php (_process_submission() calls confirm_sesskey() whenever the form marker is present). The three external/AJAX functions all require moodle/site:config both in their service declaration and via an internal require_capability() plus validate_context().

Data handling is sound throughout: all database access goes through the $DB API with bound parameters, including the one place where filter SQL fragments are concatenated — those fragments are produced only by the plugin's own filter classes using get_in_or_equal(), sql_like(), and sql_equal(), and user-facing column names are constrained to validated allow-lists. Output is consistently escaped (s(), Mustache auto-escaping, addslashes_js() for the JS context, base64 for JSON embedded in JS). Destructive operations delegate to core APIs (delete_user(), user_update_user(), cohort_add_member()/cohort_remove_member(), the badges privacy provider, and the file storage API). Site administrators and the guest account are unconditionally excluded from selection. The plugin ships a complete Privacy API implementation, uses transactions for multi-step writes, confines all DDL to db/upgrade.php, and is accompanied by extensive unit tests.

No security vulnerabilities were identified. The only issues are a single low-severity code-quality item (the anonymize action writes directly to the user table, hand-maintaining the list of PII columns to scrub) and one informational maintainability observation (regex-based rewriting of SQL bind-parameter names). Both are admin-context only and carry no exploitation path.

AI Summary

tool_userautodelete automates user notification, suspension, anonymization, and deletion via admin-defined workflows built from filter and action sub-plugins, executed by scheduled tasks.

Security posture: strong. The review found no security vulnerabilities. Despite the destructive nature of the plugin, the attack surface is tightly controlled:

  • Access control — every page and AJAX endpoint requires site-administrator privileges (require_admin() / admin_externalpage_setup() / moodle/site:config). There is no code path reachable by students, teachers, managers, or unauthenticated users.
  • CSRF — state-changing actions use require_sesskey() or Moodle form classes; the is_submitted() pattern in the management handlers is safe because moodleform enforces the sesskey at construction (verified in core).
  • SQL — all queries are parameterized; concatenated filter fragments are plugin-generated and bound, and column names are validated against allow-lists.
  • XSS — templates rely on Mustache auto-escaping, s(), and addslashes_js(); admin-entered titles are PARAM_TEXT.
  • Destructive operations — delegated to core APIs; site admins and guest are always protected from selection.

Findings:

SeverityFinding
LowAnonymize action writes directly to the user table, bypassing the core user-update API used elsewhere
InfoFragile regex-based renaming of SQL bind-parameter names in generate_user_filter_clause()

The plugin also ships a full Privacy API implementation, null-providers for every sub-plugin, transactional writes, DDL confined to db/upgrade.php, and broad PHPUnit coverage.

Findings

code qualityLow
Anonymize action writes directly to the user table instead of the core user-update API

The anonymize action scrubs personal data by calling $DB->update_record('user', ...) directly, writing a hardcoded list of columns. This bypasses the core user-update API (user_update_user() / \core\user::update_user()) that the plugin's own suspend, unsuspend, and profilefield actions use.

The write itself is safe: all values are either constant or derived from the integer userid, and update_record() binds every value, so there is no SQL injection and no untrusted input involved. The action is also only ever reached from an admin-configured workflow running in cron, and it deliberately replays the relevant side effects (\core\session\manager::destroy_user_sessions() and the \core\event\user_updated event).

The concern is maintainability and completeness, not exploitation:

  • The list of PII columns to clear is hand-maintained. If a future Moodle release adds a new personal-data field to the user table, this list will not be updated automatically, and that field would silently survive anonymization — undermining the plugin's GDPR purpose.
  • Bypassing the core API also skips any normalization or validation that core performs, so the two code paths (this action versus the other actions) can diverge in behavior over time.

The code comment explains the intent (operating on an already-deleted record, where the standard API may refuse or alter the update), which is a legitimate reason to bypass the API. This is why the item is low severity rather than a correctness bug.

Risk Assessment

Low risk. There is no attacker and no untrusted data: the operation is admin-configured, runs in cron, and writes only constant or integer-derived values through a parameterized update_record(). The realistic failure mode is forward-looking data completeness — if Moodle later adds a personal-data column to the user table, the hardcoded list will not scrub it, weakening anonymization after a core upgrade. Blast radius is limited to the fields enumerated here; session invalidation and the user_updated event are already replayed, so downstream consumers are notified. This is a code-quality/maintainability finding, not a security weakness.

Context

The anonymize action is one of the action sub-plugins attached to workflow steps. It runs from process::transition() / process::create() during the executeworkflows scheduled task, after the admin has configured and activated a workflow. In the default workflow it runs in the same step as the delete action (delete first, then anonymize), so it typically operates on a record that delete_user() has already marked deleted = 1 — which is likely why the author chose to bypass user_update_user(). All other actions that modify the user record (suspend, unsuspend, profilefield) instead route through user_update_user() / \core\user::update_user().

Identified Code
        $success = $DB->update_record('user', [
            'id' => $process->userid,
            'username' => "DELETED-USER-{$process->userid}",
            'password' => AUTH_PASSWORD_NOT_CACHED,
            'idnumber' => '',
            'firstname' => 'DELETED',
            'lastname' => 'DELETED',
            'email' => "DELETED-USER-{$process->userid}@localhost",
            'phone1' => '',
            'phone2' => '',
            'institution' => '',
            'department' => '',
            'address' => '',
            'city' => '',
            'country' => '',
            'lastip' => '',
            'secret' => '',
            'picture' => 0,
            'description' => '',
            'imagealt' => '',
            'lastnamephonetic' => '',
            'firstnamephonetic' => '',
            'middlename' => '',
            'alternatename' => '',
            'moodlenetprofile' => '',
            'timemodified' => time(),
        ]);
Suggested Fix

Option A — keep the direct write but make the field list resilient. Derive the set of PII fields to blank from a single source of truth (for example by iterating a defined list, or by starting from the core user record's keys and excluding system fields) so a core schema change is less likely to leave data behind.

Option B — document and test the coupling. Add a unit test that asserts every user-table PII column is cleared after anonymization, so that a future core field addition surfaces as a test failure rather than silent data retention.

In all cases, keep the explanatory comment describing why user_update_user() is intentionally bypassed for already-deleted users.

best practiceInfo
SQL bind-parameter collision avoidance relies on regex rewriting of the query string

step::generate_user_filter_clause() combines the SQL fragments returned by each filter instance into a single WHERE clause. To avoid bind-parameter name collisions when the same filter type appears twice in one step (e.g. two lastaccess filters that both use :lastaccesstime, or two auth filters), it rewrites every placeholder in each fragment by running preg_replace() over the raw SQL string and prefixing the parameter name with the filter's ID.

This works correctly for all shipped filters, but manipulating SQL text with a regular expression to rename bind parameters is fragile and hard to reason about:

  • It depends on the ungreedy U modifier and on a trailing space being appended so that a parameter at the very end of the fragment is followed by a non-word character.
  • preg_quote() is applied to $newparamname inside the replacement string, where preg_quote is not the correct escaping function (replacement strings treat $ and \ specially). This is harmless today only because generated parameter names are alphanumeric, but it is a latent trap if naming conventions ever change.
  • A filter author who introduces an unusual placeholder name, a placeholder whose name is a prefix of another, or SQL containing a literal colon could subtly break the rewrite.

None of these are defects in the current code — every shipped filter uses simple alphanumeric placeholders via get_in_or_equal() or fixed names, and the values are always bound, so there is no SQL-injection exposure. This is purely a robustness/maintainability observation.

Risk Assessment

Informational. No security impact: all parameter values remain bound, and the fragments being rewritten originate from the plugin's own filter classes rather than from request input. The practical risk is future maintenance — a new or third-party filter sub-plugin using an unexpected placeholder name could cause the rewrite to misfire and produce a malformed query (a runtime error, not an injection). Replacing the regex with key-based renaming or source-side prefixing would remove the fragility entirely.

Context

generate_user_filter_clause() is invoked when building the ingestion query (workflow::get_applicable_users()), the transition query (process::get_active_processes_for_step()), and the dry-run table. Each filter's user_records_filter_clause() returns a userfilter_clause with an SQL fragment and a bound-parameter array; this loop merges them while de-duplicating parameter names. The merged clause is then concatenated into a larger parameterized query and executed with the combined parameter array, so the actual values are never interpolated into SQL.

Identified Code
            $clausesql = $clause->sql;
            foreach ($clause->params as $paramname => $paramvalue) {
                $newparamname = "f{$filter->id}{$paramname}";
                $clausesql = preg_replace(
                    '/(.*:)' . preg_quote($paramname, '/') . '(\W.*)/U', // Note the 'U' for making .* ungreedy!
                    '$1' . preg_quote($newparamname, '/') . '$2',
                    $clausesql . ' '  // Append space to ensure regex detects parameters at string end.
                );

                $filterparams[$newparamname] = $paramvalue;
            }
Suggested Fix

Prefer a mechanism that does not parse SQL with a regex. Two robust alternatives:

  • Push uniqueness to the source. Have each filter build its placeholders with a caller-supplied unique prefix (most filters already accept a prefix argument to get_in_or_equal()), so no post-hoc renaming is needed.
  • Rename by key, not by text. Build the combined parameter array by prefixing the keys of $clause->params and perform a targeted str_replace(':'.$old, ':'.$new, $sql) per parameter (longest-name-first) rather than a regex over the whole string.

If the regex approach is retained, drop preg_quote() from the replacement argument (use the raw $newparamname, which is already alphanumeric) and keep the behavior covered by unit tests that include two instances of the same filter type in one step.

Additional AI Notes

Forward-compatibility branch not verifiable in this environment. The suspend, unsuspend, and profilefield actions select the user-update API by Moodle branch: $CFG->branch <= 502 ? user_update_user(...) : \core\user::update_user(...). On the reviewed core (5.2, branch 502) the user_update_user() path is taken and is correct. The \core\user::update_user() path targets Moodle 5.3 (the plugin advertises support up to 503), and that method does not exist in the 5.2 source tree available here (/moodle/public/lib/classes/user.php only defines update_picture). This is not a defect on 5.2 — the branch is dead code there — but the 5.3 path could not be verified. Recommend confirming the \core\user::update_user() signature and availability on a real Moodle 5.3 instance before release.

Strong defensive design worth highlighting. The plugin consistently protects against the most dangerous failure mode for a bulk-deletion tool: site administrators and the guest account are unconditionally excluded from user selection (step::generate_user_filter_clause()), a workflow must pass is_valid() (at least one valid filter and action per step) before it can be activated or processed, each user may only be in one active process at a time, and deactivating or deleting a workflow aborts its active processes inside a transaction. The dryrun.php preview lets administrators verify the selection before enabling a workflow.

No bundled third-party code. The plugin contains no vendor directory or bundled libraries; the files under amd/build/ are the compiled (minified) output of the plugin's own amd/src/ modules, so a thirdpartylibs.xml is correctly absent. The poetry.lock/pyproject.toml/mkdocs.yml files relate only to the documentation site build and are not shipped runtime code.

Privacy and data-cleanup coverage is complete. The main component implements the full Privacy API (metadata, context, userlist, export, and delete providers) for the process and workflow tables, and every sub-plugin ships a null_provider. All plugin data lives in the plugin's own tables, so the empty db/uninstall.php is appropriate.

This review was generated by an AI system and may contain inaccuracies. Findings should be verified by a human reviewer before acting on them.