Seasonal Animations
local_oc_seasonal_animations
- #2Season preview route enforces only login, not a site-administrator capability
- #3$plugin->requires understates the minimum supported Moodle version
- #4Admin preview link built with new moodle_url() instead of moodle_url::routed_path()
- #5Missing GPL boilerplate header in three PHP files
- #6Fragile control flow and unchecked array keys in admin_setting_random_span
A local plugin that renders decorative seasonal particle animations (snowflakes, leaves, blossoms, bubbles) on the Moodle front page (site-index) and dashboard (my-index). It hooks into before_footer_html_generation to inject a custom <snow-effect> web component and a per-user toggle button. Effects are configured per season (seasonless/spring/summer/autumn/winter) through admin settings, with an admin-only preview page built on the Moodle 4.5+ attribute-based routing API. All animation logic runs client-side in vanilla-JS AMD modules; the server stores only plugin configuration and admin-uploaded particle images (via the File API).
The plugin has a clean security posture. There is no direct database access (no $DB usage at all), no raw SQL, no superglobal access, no shell/eval execution, and no raw HTTP or filesystem access — uploaded particle images are served correctly through the File API (get_file_storage(), send_stored_file()), output is escaped via html_writer/Mustache/get_string(), and the null_provider privacy implementation is accurate because the only client state (the on/off toggle) lives in sessionStorage.
The ceiling is held below an A primarily by a feature-breaking functional defect: season::is_animaton_enabled() calls get_config() with its arguments reversed, so it always returns false and the seasonal effect is never rendered on the front page or dashboard — the plugin's central advertised feature. This is a correctness bug rather than a security risk, but it is significant and the plugin's own Behat scenarios for that code path would fail because of it.
The remaining issues are low severity and form a pattern of quality/compatibility gaps: the routed preview page enforces only require_login and no site-admin capability (low impact — it exposes only decorative, non-sensitive animation content), $plugin->requires understates the true minimum Moodle version (declares 4.0 while depending on 4.5-era routing, core\output\html_writer, and PHP 8.1 enums), the admin preview link is built with new moodle_url() instead of moodle_url::routed_path() (so it 404s on default installs where routerconfigured is false), three PHP files omit the GPL boilerplate header, and one admin setting has fragile control flow. No high or critical findings were identified.
Overview
local_oc_seasonal_animations is a small, well-structured presentation plugin that overlays animated particles on the Moodle front page and dashboard. The codebase is modern (PHP 8.1 enums, the 4.5+ routing API, the Hook API) and demonstrates good Moodle hygiene in the areas that matter most for security.
Security assessment
The plugin is clean from a security standpoint:
- No data-layer risk — there is no
$DBusage, no SQL, no schema manipulation, and no direct filesystem/datarootaccess. Admin-uploaded particle images are stored and served entirely through the File API (admin_setting_configstoredfile,get_area_files(),get_file_by_hash(),send_stored_file()), and thepluginfilecallback validates context level and file-area naming before serving. - No injection surface — no superglobals, no
eval/exec/shell_exec, no raw cURL/file_get_contentson URLs. HTML output goes throughhtml_writer, Mustache auto-escaping, andget_string(). The JS reads only admin-controlled animation configuration. - Privacy is correct — the
null_provideris accurate because the only user state (the effect on/off toggle) is held in browsersessionStorage, never server-side.
What lowers the grade
The headline issue is functional, not security related:
season::is_animaton_enabled()callsget_config("{$this->value}_enabled", 'local_oc_seasonal_animations')— the component and setting-name arguments are swapped. Moodle's signature isget_config($plugin, $name), so this always returnsfalseand the footer hook short-circuits, meaning the effect never appears on the front page or dashboard.
A cluster of low-severity items rounds out the review: the routed preview page lacks a site-admin capability check (low impact — decorative content only), $plugin->requires is set to Moodle 4.0 despite hard dependencies on Moodle 4.5 APIs, the admin preview URL is constructed without moodle_url::routed_path() (broken on default installs), three files miss the GPL header, and one admin setting has fragile parsing logic.
Bottom line
Security practices are sound; the plugin would benefit most from fixing the get_config() argument order (which restores its core functionality and unblocks its Behat suite) and correcting the version requirement before release.
Findings
season::is_animaton_enabled() invokes Moodle's get_config() with its two arguments in the wrong order. The documented signature is get_config($plugin, $name) — component name first, setting name second. This method instead passes the setting name ("{$this->value}_enabled", e.g. seasonless_enabled) as the component and the component (local_oc_seasonal_animations) as the setting name.
Because config_plugins contains no plugin named seasonless_enabled (or winter_enabled, etc.), the two-argument form of get_config() returns false in every case. Consequently is_animaton_enabled() always evaluates to false for all five seasons.
The footer hook gates all rendering on this method: hook_callbacks::before_footer_html_generation() calls if (!$season->is_animaton_enabled()) { return; } before injecting the effect. With the method permanently false, the <snow-effect> element, the toggle button, and the AMD initialisation are never added to the front page (site-index) or dashboard (my-index) — i.e. the plugin's primary advertised behaviour does not work.
The inconsistency is visible within the same file: get_current_season() reads get_config('local_oc_seasonal_animations', 'season_change_enabled') (correct order), while is_animaton_enabled() reverses it. This is a functional correctness defect with no security impact.
Medium risk (functional defect, not a security issue). Blast radius is every installation that enables the plugin and relies on the documented front-page/dashboard behaviour — the feature is silently inert there. There is no data, privacy, or access-control impact. It is rated medium rather than low because it breaks the plugin's core purpose and would fail its own automated tests, making it the most consequential finding even though it carries no security risk.
is_animaton_enabled() is the sole gate on the only production render path (the before_footer_html_generation hook). The preview controller (preview_controller::index()) calls seasonal_effect::render() directly and does not consult is_animaton_enabled(), so the admin preview still works — which likely explains why the defect went unnoticed during manual testing. The plugin's own Behat scenarios in tests/behat/toggle_animation.feature, tests/behat/particle_types.feature, and tests/behat/start_position_types.feature set seasonless_enabled = 1 under the local_oc_seasonal_animations plugin and then assert that particle elements exist on the homepage; with the arguments swapped, those elements are never created, so these scenarios cannot pass.
public function is_animaton_enabled(): bool {
return get_config("{$this->value}_enabled", 'local_oc_seasonal_animations');
}
Swap the arguments so the component is passed first and cast the result to a strict boolean:
public function is_animaton_enabled(): bool {
return (bool) get_config('local_oc_seasonal_animations', "{$this->value}_enabled");
}
The preview endpoint is exposed through the routing API with requirelogin: new require_login(requirelogin: true, ...). Core's \core\router\require_login only governs authentication (login / course login / guest autologin); it performs no capability check. The controller body (index()) also contains no is_siteadmin() or require_capability() call.
The page is clearly intended to be administrator-only: it is registered in settings.php as an admin_externalpage and linked only from within Site administration. However, the underlying route /local_oc_seasonal_animations/preview/{season} is directly reachable by any authenticated user (and, since autologinguest defaults to true, by guests where guest access is enabled). The admin_externalpage registration controls only the visibility of the admin-tree link, not access to the target URL.
Low risk. The authorization gap is real — an admin-intended page is reachable by any logged-in user (and possibly guests) — but the impact is negligible because the page is read-only and exposes only decorative, non-sensitive animation settings and image URLs. There is no data modification, no state change, and no disclosure of other users' data. It is worth tightening for correctness and defence-in-depth rather than because it enables meaningful harm.
init_page() sets the system context and a standard page layout, then the controller writes $OUTPUT->header(), the seasonal effect markup, and $OUTPUT->footer() to the PSR-7 response. The only data surfaced to the client is the season's animation configuration (get_animation_configs()) plus the public pluginfile URLs of admin-uploaded particle images — none of which is sensitive, and all of which is already shown to every visitor on the front page when the effect is active.
Authenticated as a regular student, request the routed URL (default install, routerconfigured = false):
GET https://<site>/r.php/local_oc_seasonal_animations/preview/winter
The page renders the seasonal preview (header, animation, footer) even though the user has no administrative rights.
#[route(
path: '/preview/{season}',
method: ['GET'],
pathtypes: [new path_season_animation()],
requirelogin: new require_login(
requirelogin: true,
courseattributename: 'course',
),
)]
public function index(
season $season,
ServerRequestInterface $request,
ResponseInterface $response,
): ResponseInterface {
Add an explicit administrator/capability check at the start of index(), since the page is admin tooling:
public function index(
season $season,
ServerRequestInterface $request,
ResponseInterface $response,
): ResponseInterface {
require_capability('moodle/site:config', context_system::instance());
// ... existing body ...
}
If broader access is genuinely intended, that should be a deliberate decision; otherwise gate it to moodle/site:config (or a dedicated plugin capability).
version.php declares $plugin->requires = 2022041900, which corresponds to Moodle 4.0. The plugin, however, depends on APIs and language features that did not exist until Moodle 4.5:
- The attribute-based routing system (
\core\router\route,\core\router\require_login,path_parameter, etc.) used bypreview_controllerandpath_season_animation— introduced in Moodle 4.5. \core\output\html_writer(used inseasonal_effect.php) — this autoloaded class location was added in Moodle 4.5; earlier versions only have the global\html_writer.- PHP 8.1 enums (
enum season: string) — Moodle 4.0 supports PHP 7.3/7.4/8.0.
Installing on Moodle 4.0–4.4 would therefore produce fatal errors (unresolvable classes, enum parse errors) rather than a graceful "requirements not met" message. The bundled CI already targets MOODLE_405_STABLE, confirming 4.5 as the real floor.
Low risk. This is a compatibility/metadata defect. The practical consequence is that administrators on Moodle 4.0–4.4 can install the plugin and then hit fatal errors instead of being told the version is unsupported. No security impact.
Moodle uses $plugin->requires during installation/upgrade to refuse installation on older cores. An incorrect (too-low) value defeats this safety net, allowing the plugin to be installed where its dependencies are absent.
$plugin->requires = 2022041900;
Raise the requirement to at least the Moodle 4.5 release:
$plugin->requires = 2024100700; // Moodle 4.5 — routing API, \core\output\html_writer, PHP 8.1 enums.
Optionally also declare $plugin->supported / $plugin->dependencies if you want to formally cap the supported range.
In settings.php the preview page URL is constructed with new moodle_url('/local_oc_seasonal_animations/preview/' . $setinfos[1]). For paths served by the new router, core provides moodle_url::routed_path() specifically because routed URLs must be prefixed with /r.php/ whenever $CFG->routerconfigured is false.
$CFG->routerconfigured defaults to false (confirmed in config-dist.php, which notes "From Moodle 4.5 this is set to false ... it must be accessed via .../r.php"). A plain new moodle_url('/local_oc_seasonal_animations/preview/spring') therefore yields https://<site>/local_oc_seasonal_animations/preview/spring, with no /r.php/ segment — which does not resolve on a default install, so the admin "Preview" action icon leads to a 404.
Low risk. Purely a functional/usability defect affecting administrators: the in-admin preview link does not work unless the site operator has enabled clean routing (routerconfigured = true). No security or data impact.
The URL is passed to an admin_externalpage and rendered as an edit/preview action icon on each season's general settings page. On an install with default routing configuration the generated link points to a non-routed path.
$previewpage = new moodle_url('/local_oc_seasonal_animations/preview/' . $setinfos[1]);
Use the routed-path helper so the correct prefix is applied based on $CFG->routerconfigured:
$previewpage = moodle_url::routed_path('/local_oc_seasonal_animations/preview/' . $setinfos[1]);
Moodle's coding style requires every PHP file to begin with the standard GPL license boilerplate (// This file is part of Moodle ...). Three files omit it entirely and start directly with <?php followed by the namespace declaration. The remaining PHP files in the plugin include the header correctly, so this is an inconsistency the moodle.Files.BoilerplateComment sniff in the bundled phpcs CI step would flag.
Low risk. Coding-standard / licensing-hygiene violation with no functional or security consequence, but it would fail the plugin's own phpcs gate.
These are autoloaded class files; the missing header is a coding-standards issue only and does not affect behaviour.
<?php
namespace local_oc_seasonal_animations;
Prepend the standard Moodle GPL boilerplate block above the namespace line (the same block already present in, e.g., classes/seasonal_effect.php).
<?php
namespace local_oc_seasonal_animations\route\controller;
Prepend the standard Moodle GPL boilerplate block; also add a class-level docblock for completeness.
<?php
namespace local_oc_seasonal_animations\route\utils;
Prepend the standard Moodle GPL boilerplate block above the namespace line.
Two minor robustness issues exist in admin_setting_random_span:
- In
output_html()the branch that should beelse ifis written as a bareifafter the preceding block (} if (!is_array($data)) {). It happens to behave correctly today only because the earlier branches always leave$dataas an array, making theValueErrorbranch effectively unreachable. This is a latent logic error — a future edit to the branches above could turn it into a misfire. - In
validate(),$data['min']and$data['max']are read withtrim()without first confirming the keys exist. If a submission ever omits one of them, PHP 8 raises an "Undefined array key" warning. In practice the Mustache form always submits both fields, so this is defensive only.
Low risk. Both issues are latent — they do not cause incorrect behaviour given the current form and data flow, and the setting is admin-only. They are worth tidying for maintainability and to avoid future regressions, but pose no security or data risk.
This custom admin setting stores a min/max pair as a "min;delta" string. It is reachable only by site administrators through the settings UI, which always posts both min and max inputs from the setting_random_span template.
} if (!is_array($data)) {
throw new ValueError("Value cannot be parsed");
}
Make the intent explicit by chaining the condition:
} else if (!is_array($data)) {
throw new ValueError("Value cannot be parsed");
}
$min = trim($data['min']);
$max = trim($data['max']);
Guard the keys before use:
if (!isset($data['min'], $data['max'])) {
return get_string('validateerror', 'admin');
}
$min = trim($data['min']);
$max = trim($data['max']);
README documents a non-existent preview URL. README.md instructs admins to open /local/local_oc_seasonal_animations/preview.php?season=winter_, but there is no preview.php script and the season values do not carry a trailing underscore in the route. The actual endpoint is the routed path /local_oc_seasonal_animations/preview/<season> (served via /r.php/... on default installs), with values seasonless, spring, summer, autumn, winter. Updating the documentation would avoid confusing administrators.
The plugin's own Behat suite is coupled to finding #1. toggle_animation.feature, particle_types.feature, and start_position_types.feature configure seasonless_enabled = 1 under the local_oc_seasonal_animations plugin and then assert that particle elements appear on the homepage. Because is_animaton_enabled() reads that setting with swapped arguments, these scenarios should currently fail — fixing the get_config() argument order is what makes both the feature and its tests work again.
Positive: security fundamentals are well handled. The plugin avoids every common Moodle security pitfall — there is no $DB/SQL usage, no superglobals, no eval/exec/shell, and no raw HTTP or dataroot/filedir access. Admin-uploaded particle images go through the File API end-to-end (admin_setting_configstoredfile with accepted_types => ['image'], size/file caps, served via send_stored_file() after context and file-area validation), output is escaped through html_writer/Mustache/get_string(), and the null_provider privacy implementation is correct because the on/off toggle is persisted only in browser sessionStorage.
Image-particle CSS uses an interpolated background URL. In amd/src/local/dashboard_effect/particle/image_particle.js, this.style.backgroundImage = \url(${url})`uses a server-generated pluginfile URL derived from admin-uploaded filenames. This is not exploitable by non-admins (the value is system/admin-controlled), but usingCSS.escape()` or assigning via a known-safe URL would be marginally more robust against unusual filenames.