MDL Shield

Information Center

local_information_center

Print Report
Plugin Information

A local plugin ("Information Centre") that manages and displays role-targeted notification messages inside Moodle. It provides an admin control board (create / edit / soft-delete), a per-user inbox with search, category filters and read tracking, a scheduled cleanup task, and HTTP API endpoints built on Moodle 5.x's core\router framework. Visibility is tiered by role (student / teacher / manager / admin) and enforced through capabilities.

Version:2026061200
Release:2.0.2
Reviewed for:5.2
Privacy API
Unit Tests
Behat Tests
Reviewed:2026-07-13
50 files·5,364 lines
Grade Justification

The plugin demonstrates strong security fundamentals. Every database access uses parameterized $DB queries or Moodle helpers (get_in_or_equal, sql_like); every route enforces require_login plus an explicit capability check; the destructive delete route validates require_sesskey(); edit uses moodleform (which handles sesskey automatically); message ownership is enforced on both update and delete; user-facing output is escaped via format_string and format_text; and a complete Privacy API implementation is present.

The most notable issue is a missing sesskey check on the renotify API route, which accepts POST/PUT and changes state (resets read status for all recipients) without CSRF protection. Its real-world impact is limited — it only re-notifies users, requires a victim holding the update_or_create_messages capability, and requires knowledge of an unguessable message UUID — so it is assessed as low rather than medium.

The remaining findings are all low or informational: the admin table prints subject without an explicit output-escape (not currently exploitable because PARAM_TEXT strips tags on every write path), a privacy-metadata string/field mismatch, generic \Exception usage with hardcoded English messages, an under-declared $plugin->requires version relative to the 5.x APIs actually used, broken test fixtures referencing a removed column, and a couple of minor code-quality nits. No high or critical vulnerabilities were found; a regular authenticated user cannot access, modify, or damage other users' data.

AI Summary

Overview

local_information_center is a cleanly architected Moodle 5.x local plugin for publishing role-targeted "information centre" messages. It uses modern core facilities: the core\router HTTP routing framework, dependency injection (core\di + a di_configuration hook), typed contracts/interfaces, an enum for visibility, and scheduled tasks.

Security posture

The security-critical paths are handled well:

  • SQL — all queries are parameterized ($DB placeholders, get_in_or_equal, sql_like with sql_like_escape). No string concatenation of user input into SQL was found.
  • Access control — every route calls require_login (via router metadata) and an explicit require_capability / has_any_capability check. Visibility is filtered per-user against the caller's effective rights.
  • CSRF — the delete route calls require_sesskey(); the edit route uses moodleform (automatic sesskey); the JSON add_or_update API is protected by its application/json content-type requirement.
  • Output — the user inbox escapes titles with format_string and passes bodies through format_text (which purifies untrusted HTML). Mustache auto-escaping is used for user-influenced values.
  • Privacy — a full GDPR provider (metadata, export, delete-per-user, delete-in-context, userlist) is implemented.
  • Ownership — updates and deletes verify useridfrom === $USER->id.

Weak spots

  • The renotify API route changes state on POST/PUT without a sesskey check (low-impact CSRF).
  • The admin control board prints subject without an explicit output escape, relying on upstream PARAM_TEXT cleaning (currently safe, but fragile defense-in-depth).
  • Minor compliance and code-quality issues: a privacy-metadata string/field mismatch, generic exceptions with hardcoded strings, an under-declared requires version, and broken unit-test fixtures.

Bottom line

A solid, security-conscious plugin. The findings are low severity and mostly hygiene; addressing the renotify sesskey gap and the privacy-metadata mismatch would bring it close to exemplary.

Findings

securityLow
State-changing `renotify` API route has no sesskey / CSRF protection
Exploitable by:
manager

The renotify routing endpoint is registered for POST and PUT and performs a state-changing action — it deletes every read-marker for a message (reset_readcount), causing the message to reappear as unread for all recipients. The handler enforces require_login and require_capability('local/information_center:update_or_create_messages'), but performs no sesskey / CSRF token validation.

Moodle's core\router framework does not add CSRF protection automatically — its authentication middleware only calls require_login(). Any state-changing route reached via the session cookie must therefore call require_sesskey() itself (as the sibling delete route correctly does). Because this route has no request body, it imposes no application/json content-type constraint, so a plain cross-site POST form can reach it.

The companion add_or_update API route (/notifications/{uuid}) is not affected in the same way, because its required application/json request body makes it non-trivial to forge from a browser.

Secondary observation: unlike edit and delete, renotify also performs no ownership check, so any holder of the capability can reset the read state of a message they do not own.

Risk Assessment

Low risk. This maps to the classic "missing CSRF protection where other controls exist" pattern, which is often medium; here it is assessed low because the concrete impact is minimal and exploitation is constrained:

  • Impact: only resets read/unread state (a notification nuisance). No data is disclosed, modified, or destroyed, and no privilege boundary is crossed.
  • Accessibility: the victim must hold update_or_create_messages (no default archetype — effectively managers/admins), and the attacker must already know a target message's random UUID, which low-privilege users cannot normally observe.

The fix is trivial (require_sesskey()), so it is worth closing despite the limited blast radius.

Context

Routes in this plugin are dispatched by core\router. The framework's moodle_authentication_middleware only invokes require_login() for routes whose requirelogin metadata is set; it never checks sesskey. The plugin's own delete route (notification_controller::delete) demonstrates the expected pattern by calling require_sesskey() explicitly and by embedding sesskey() into the delete URL (paths::delete_notification). The renotify route omits both.

Proof of Concept

An attacker hosts a page that auto-submits a cross-site POST. When a logged-in user holding update_or_create_messages (typically a manager) visits it, the read status of the target message is reset and every recipient is re-notified:

<form id="f" method="POST"
      action="https://moodle.example/r.php/local_information_center/messages/<known-uuid>/renotify"></form>
<script>document.getElementById('f').submit()</script>

(The /r.php/ prefix is used when the site router is not front-controller-configured; otherwise the clean path /local_information_center/messages/<uuid>/renotify applies.) The attacker must know a valid message UUID, which is a random v4 value and not enumerable.

Identified Code
#[route(
    title: 'Notification Renotify',
    description: 'Reset the read status of a notification',
    path: '/messages/{uuid}/renotify',
    method: ['PUT', 'POST'],
    pathtypes: [new notification_id(true)],
    responses: [new ok_response()],
    requirelogin: new require_login()
)]
public function renotify(
    string $uuid,
    ServerRequestInterface $request,
    NotificationsRead $readstatusmanager,
): payload_response {
    global $PAGE;

    $ctx = system::instance();
    $PAGE->set_context($ctx);
    require_capability('local/information_center:update_or_create_messages', $ctx);

    $readstatusmanager->reset_readcount($uuid);
Suggested Fix

Validate the sesskey before mutating state, mirroring the delete controller:

require_capability('local/information_center:update_or_create_messages', $ctx);
require_sesskey();

$readstatusmanager->reset_readcount($uuid);

Ensure every client that calls this endpoint sends the current sesskey (as a parameter or the X-CSRFToken-equivalent Moodle expects). Consider also restricting renotify to the message owner, consistent with edit/delete.

best practiceLow
Admin control board outputs `subject` without an explicit output escape

In the admin control board, notification_admin_table::add_notification_data() places the raw subject value into a table cell. Moodle's flexible_table writes cell content verbatim (html_writer::tag('td', $content) does not HTML-encode), so the plugin — not core — is responsible for escaping.

Every other user-influenced value in this method is escaped (the author name is wrapped in s()), and the user-facing inbox escapes the same field with format_string. Only this admin table prints subject unescaped.

This is not currently exploitable: subject is cleaned with PARAM_TEXT on both write paths (the edit form silently strip_tags-es it; the JSON API's scalar_type(param::TEXT) validation rejects any value that changes under cleaning), so a stored value can never contain </>. It is reported as defense-in-depth: the safety relies on an undocumented invariant that all future write paths keep using PARAM_TEXT. Escaping on output removes that dependency.

Risk Assessment

Low risk. No exploit exists today because PARAM_TEXT strips or rejects HTML tags on every path that can write subject, and the value is rendered as element text (not inside an attribute). The concern is purely defensive: output escaping should not depend on an implicit input-cleaning invariant. Blast radius, were the invariant ever broken, would be limited to other privileged staff viewing the board.

Context

The control board is rendered in notification_controller::index(), which buffers the table output and echoes it to the response. It is reachable only by holders of can_view_message_control_board. Messages are authored by holders of update_or_create_messages via the edit form (title is PARAM_TEXT) or the JSON API (subject is scalar_type(param::TEXT)).

Identified Code
$this->add_data([
    $notification->subject,
    $timestart,
    $timeend,
    $userlink,
    get_string("category:$notification->categoryname", 'local_information_center'),
    $edit,
    $delete,
]);
Suggested Fix

Escape the subject when building the row, consistent with the inbox renderer:

$this->add_data([
    format_string($notification->subject, true, ['context' => context_system::instance()]),
    $timestart,
    ...
]);

format_string() (or s()) guarantees the cell is safe regardless of how the value was stored.

complianceLow
Privacy metadata references a missing string and a non-existent field name

The Privacy API metadata does not line up with the language pack or the database schema:

  • Missing language string. get_metadata() declares the messageuuid column of the local_information_center table with the string key privacy:metadata:local_information_center:messageuuid, but the language files only define privacy:metadata:local_information_center:messageid (the pre-UUID name). The referenced string does not exist, so the privacy registry renders a [[...]] placeholder / raises a missing-string debug notice.
  • Wrong field name. For the local_information_center_messages table the metadata maps a field named userid, but that table's column is actually useridfrom (there is no userid column). The description string used is the ...:useridfrom one, so the label is fine, but the declared field name is inaccurate.

These are GDPR/compliance documentation defects rather than data-handling bugs — the actual export/delete logic operates on the correct columns.

Risk Assessment

Low risk. No user data is mishandled; the defect is inaccurate self-documentation of processed data. It should be corrected for GDPR-compliance accuracy and to avoid missing-string debug output.

Context

The provider is otherwise complete and correct: export_user_data, delete_data_for_user, delete_data_for_users, delete_data_for_all_users_in_context and get_users_in_context all operate on useridfrom and messageuuid. The mismatch is confined to the declarative metadata surfaced in Site administration → the privacy registry.

Identified Code
$collection->add_database_table(
    'local_information_center',
    [
        'userid' => 'privacy:metadata:local_information_center:userid',
        'messageuuid' => 'privacy:metadata:local_information_center:messageuuid',
    ],
    'privacy:metadata:local_information_center'
);
Suggested Fix

Add the missing string to the language files, e.g. rename messageid to messageuuid:

$string['privacy:metadata:local_information_center:messageuuid'] = 'The UUID of the notification that the user read.';
Identified Code
'userid' => 'privacy:metadata:local_information_center_messages:useridfrom',
Suggested Fix

Use the real column name so the metadata matches the schema:

'useridfrom' => 'privacy:metadata:local_information_center_messages:useridfrom',
Identified Code
$string['privacy:metadata:local_information_center:messageid'] = 'The ID of the notification.';
Suggested Fix

Replace the obsolete messageid key with messageuuid (matching provider::get_metadata), and apply the same change to lang/de/local_information_center.php.

code qualityLow
Ownership / not-found conditions thrown as generic `\Exception` with hardcoded English

Several error conditions — including security-relevant ownership checks — are raised with the base \Exception class and, in some cases, hardcoded English strings rather than a Moodle exception type (moodle_exception / coding_exception) and get_string().

Consequences:

  • User-visible messages are not translatable and bypass the language pack.
  • Base \Exception is not caught by Moodle's typed exception handling and produces a less consistent error page / HTTP status than moodle_exception.
  • It is harder for callers to distinguish an authorization failure from an unexpected error.
Risk Assessment

Low risk. No security impact — the guards work. This is a code-quality / i18n concern: hardcoded English strings violate Moodle's translation requirement, and base \Exception is discouraged in favour of moodle_exception.

Context

The ownership checks themselves are correct and valuable — they prevent one privileged author from editing or deleting another author's message. The issue is purely how the failure is signalled. Note notification_manager::add_or_update() already uses get_string() for its messages but still wraps them in a base \Exception.

Identified Code
$message = $messagemanager->get($uuid);
if (!$message) {
    throw new Exception("Message $uuid not found");
}
if ($message->useridfrom !== (int) $USER->id) {
    throw new Exception('You are not the owner of this message');
}
Suggested Fix

Use a Moodle exception and language strings, e.g.:

if (!$message) {
    throw new \moodle_exception('notfound', 'local_information_center');
}
if ($message->useridfrom !== (int) $USER->id) {
    throw new \required_capability_exception($context, ..., 'nopermission', '');
    // or a dedicated moodle_exception with a lang string
}
Identified Code
if ((int) $record->useridfrom !== (int) $USER->id) {
    throw new Exception('You are not the owner of this message');
}
Suggested Fix

Throw a moodle_exception built from a language string (a validation:* key already exists for the sibling "owner cannot be changed" case).

code qualityLow
`add_filter()` "isset" branch uses the raw field key instead of the mapped column

In notification_admin_table::add_filter(), the isset comparator builds two different SQL fragments. The "not null" branch correctly uses the whitelisted, table-qualified $column (from FIELD_MAP), but the "is null" branch uses the raw $field key instead, producing an unqualified identifier (e.g. timedeleted IS NULL rather than m.timedeleted IS NULL).

This is a latent bug, not an injection vector: $field has already been validated against FIELD_MAP at the top of the method, so it can only be one of a fixed set of keys. It happens to work today because timedeleted is unambiguous across the joined tables, but it is inconsistent and would break if a field name ever collided across the join.

Risk Assessment

Low risk. No security impact (whitelisted identifier, parameterized values). It is a correctness/consistency defect that could surface as a SQL error only if the schema evolved to introduce an ambiguous column name.

Context

add_filter() is fed by notification_filter_form::set_filters(). The timedeleted filter is always submitted with the isset comparator, so the buggy branch is exercised whenever the "show deleted" checkbox is off. Because $field/$column are constrained to FIELD_MAP keys and all filter values are bound as ? placeholders, there is no SQL-injection exposure.

Identified Code
case 'isset':
    if ($value) {
        $this->filters[] = "$column IS NOT NULL";
    } else {
        $this->filters[] = "$field IS NULL";
    }
    break;
Suggested Fix

Use the mapped column in both branches:

case 'isset':
    $this->filters[] = $value ? "$column IS NOT NULL" : "$column IS NULL";
    break;
code qualityLow
Unit/privacy tests insert into a removed `messageid` column

Two privacy tests insert rows into the local_information_center table using a messageid field. That column no longer exists — the 2026041601 upgrade step (adapt_is_read_table_to_uuid) replaced messageid with messageuuid, and db/install.xml defines only id, messageuuid, and userid. These inserts will raise a DML exception, so the affected tests fail.

Other tests in the same file correctly use messageuuid, which makes the discrepancy an oversight from the UUID migration rather than an intentional schema.

Risk Assessment

Low risk. Test-only defect with no runtime/security impact, but it means part of the privacy test suite does not actually execute successfully and therefore is not validating the provider as intended.

Context

The CI workflow (.github/workflows/moodle-ci.yml) runs PHPUnit with --fail-on-warning, so these broken fixtures would cause a red build. The production code paths themselves are consistent with messageuuid.

Identified Code
$DB->insert_record('local_information_center', (object)[
    'userid' => $user2->id,
    'messageid' => $notification->uuid,
]);
Suggested Fix

Use the current column name:

$DB->insert_record('local_information_center', (object)[
    'userid' => $user2->id,
    'messageuuid' => $notification->uuid,
]);

Apply the same fix to the second occurrence (test_delete_data_for_all_users_in_context).

Identified Code
$DB->insert_record('local_information_center', (object)[
    'userid' => $user->id,
    'messageid' => $message->uuid,
]);
Suggested Fix

Replace messageid with messageuuid here as well.

code qualityLow
JSON API does not validate presence of required body fields before use

The add_or_update API handler passes the parsed body to parse_to_notification(), which reads subject, fullmessage, visibility, and categoryid with direct array access and no existence check. The router's schema_object::validate_data() only cleans/strips supplied keys and drops unknown ones — it does not enforce that fields marked required are actually present. Consequently a well-formed JSON object that simply omits one of these keys yields an "Undefined array key" warning and then a TypeError when null is passed to the non-nullable notification::create() parameters, surfacing as an HTTP 500 instead of a clean 400/validation error.

Risk Assessment

Low risk. Not a security vulnerability — reachable only by an authorized privileged caller, and the worst outcome is a 500 error with no state change. It is a request-validation quality gap.

Context

The endpoint already requires require_login and the update_or_create_messages capability, so only privileged callers reach this code. The concern is robustness/quality: a partial payload produces an internal error rather than a well-formed validation response, and emits PHP warnings.

Identified Code
$notification = notification::create(
    $notificationdata['subject'],
    $notificationdata['fullmessage'],
    FORMAT_HTML,
    '',
    $notificationdata['visibility'],
    $notificationdata['categoryid'],
    $notificationdata['timestart'] ?? null,
    $notificationdata['timeend'] ?? null,
    'external',
    $id
);
Suggested Fix

Validate presence explicitly and fail with a client error, e.g.:

foreach (['subject', 'fullmessage', 'visibility', 'categoryid'] as $required) {
    if (!array_key_exists($required, $notificationdata)) {
        throw new invalid_parameter_exception("Missing field: {$required}");
    }
}

Alternatively, supply schema defaults or otherwise treat missing required keys as a 400-level validation failure rather than letting a TypeError escape.

code qualityLow
`$plugin->requires` is below the Moodle version providing the APIs used

version.php declares $plugin->requires = 2024100100, which is Moodle 4.5.0. However the plugin depends throughout on APIs that are part of Moodle's 5.x routing framework and hook system, including core\router\route, the router schema classes, moodle_url::routed_path(), core\hook\di_configuration, and admin_externalpage targets built from routed paths.

Installing this plugin on Moodle 4.5 would therefore fail with class-not-found / method-not-found fatals despite the declared compatibility. The declared minimum should match the lowest Moodle release that actually provides these APIs (the routing framework was introduced in Moodle 5.0 and extended in 5.1/5.2). The CI matrix only exercises MOODLE_502_STABLE, so the too-low requires value has never been validated against a 4.5 install.

Risk Assessment

Low risk. Fails safe (installation aborts on an incompatible version rather than corrupting data), but it is a real compatibility-metadata defect that misrepresents the plugin's minimum requirement.

Context

The routing framework and moodle_url::routed_path() are core 5.x additions (the plugin's own db/hooks.php, settings.php, and every controller rely on them). A site administrator trusting the declared requires could attempt installation on 4.5 and hit a hard failure.

Identified Code
$plugin->version   = 2026061200;
$plugin->requires  = 2024100100;
Suggested Fix

Raise requires to at least the Moodle 5.0 release version that introduced core\router (or, to match what CI actually tests, the Moodle 5.2 branch version). For example use the $branch/release constant of the lowest supported version rather than the 4.5 value. Optionally add a $plugin->supported = [502, 502]; range.

code qualityInfo
`can_view_message_control_board` capability declared with `captype => 'write'`

The can_view_message_control_board capability grants read access to the admin control board (it gates the notification_controller::index() listing), but it is declared with captype => 'write'. Read-oriented capabilities should use captype => 'read'. The captype mainly influences reporting/《risk》 grouping and role-definition UIs; it does not weaken the runtime check here, but the classification is inaccurate.

Risk Assessment

Informational. No functional or security impact — a metadata accuracy nit.

Context

Capability enforcement (require_capability) behaves identically regardless of captype; the value is metadata used by Moodle's role/permission tooling to categorise capabilities.

Identified Code
'local/information_center:can_view_message_control_board' => [
    'captype' => 'write',
    'contextlevel' => CONTEXT_SYSTEM,
    'archetypes' => [],
],
Suggested Fix

Use the read captype for a view capability:

'local/information_center:can_view_message_control_board' => [
    'captype' => 'read',
    'contextlevel' => CONTEXT_SYSTEM,
    'archetypes' => [],
],
Additional AI Notes

Modern architecture. The plugin is a good example of a Moodle 5.x local plugin built on the new core\router HTTP routing framework, dependency injection (core\di with a di_configuration hook binding interfaces to implementations), typed value objects/contracts, and a visibility enum. Reviewers should note that the routing framework does not provide automatic CSRF protection — each state-changing route must call require_sesskey() itself, which is the root of finding 1.

SQL safety is consistently good. All dynamic queries (notification_query, notifications_read::count_unread, notification_admin_table::get_data, the privacy provider, enrol_utils) use bound parameters, get_in_or_equal, and sql_like/sql_like_escape. The one interpolated SQL fragment (notification_query_data::$order) is a hardcoded constant, not user input, and is reset to empty for count queries.

Output escaping in the user-facing inbox is correct. Titles use format_string() with the system context, message bodies go through message_format_message_text()format_text() with trusted = false, which purifies untrusted HTML; the {{{message}}} triple-mustache therefore renders already-sanitised markup. This means a malicious author cannot store working script in the body that survives to the reader.

Visibility model. Message visibility is tiered (student ⊂ teacher ⊂ manager ⊂ admin) and computed per-user from capabilities plus site-wide teacher/editingteacher enrolment (visibility::get_users_visibility). Being enrolled as a teacher in any single course therefore grants site-wide visibility of teacher-targeted messages — this appears intentional and is documented in the help strings, but administrators should be aware of it. The query filter fails safe (get_in_or_equal(..., onemptyitems: true) yields a never-matching clause) if a user somehow has no rights.

No third-party code and no bundled libraries were found, so a thirdpartylibs.xml file is not required. The plugin does not duplicate any library shipped by Moodle core.

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