From 5b485cc5b537a62d0d3c687938e4cba3c5b716c0 Mon Sep 17 00:00:00 2001 From: erseco Date: Sat, 13 Jun 2026 09:12:23 +0100 Subject: [PATCH 01/35] feat(security): add iframemode admin setting + player_iframe sandbox policy helper --- classes/local/ui/player_iframe.php | 93 ++++++++++++++++++++++++++++++ lang/en/exelearning.php | 4 ++ settings.php | 19 ++++++ 3 files changed, 116 insertions(+) create mode 100644 classes/local/ui/player_iframe.php diff --git a/classes/local/ui/player_iframe.php b/classes/local/ui/player_iframe.php new file mode 100644 index 0000000..111e3ba --- /dev/null +++ b/classes/local/ui/player_iframe.php @@ -0,0 +1,93 @@ +. + +/** + * Package iframe security mode and sandbox policy (DEC-0059). + * + * @package mod_exelearning + * @copyright 2026 ATE (Área de Tecnología Educativa) + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace mod_exelearning\local\ui; + +/** + * Resolves the configured package iframe mode and the sandbox tokens it implies. + * + * A single site-wide admin setting (mod_exelearning/iframemode) selects how the + * arbitrary author HTML/JS of an `.elpx` package is embedded in view.php: + * + * - secure (default): the iframe drops allow-same-origin, so the package runs in + * an opaque origin and cannot read or modify Moodle's DOM, cookies or session. + * SCORM scoring is relayed to the parent over a validated postMessage bridge + * (js/scorm_bridge_shim.js in the iframe, js/scorm_bridge_relay.js in the + * parent), and the parent keeps the sesskey and performs the track.php request. + * - legacy: the historical same-origin sandbox, kept only as a compatibility + * fallback for packages that misbehave under an opaque origin. + * + * Centralised here so the token policy is unit-testable without rendering view.php. + * See research ADR DEC-0059 (advances the Tier 2 roadmap of DEC-0019). + */ +final class player_iframe { + /** @var string Secure mode: opaque-origin iframe + postMessage SCORM bridge. */ + public const MODE_SECURE = 'secure'; + + /** @var string Legacy mode: historical same-origin iframe. */ + public const MODE_LEGACY = 'legacy'; + + /** + * Resolve the configured iframe mode, defaulting to secure for any unset or + * unrecognised value (fail safe: an invalid config must not weaken isolation). + * + * @return string self::MODE_SECURE or self::MODE_LEGACY. + */ + public static function resolve_mode(): string { + $mode = get_config('mod_exelearning', 'iframemode'); + return ($mode === self::MODE_LEGACY) ? self::MODE_LEGACY : self::MODE_SECURE; + } + + /** + * Whether the configured mode isolates the package (opaque origin + bridge). + * + * @return bool True in secure mode. + */ + public static function is_secure(): bool { + return self::resolve_mode() === self::MODE_SECURE; + } + + /** + * Sandbox token list for a given mode. + * + * Both modes deliberately OMIT allow-top-navigation (a package must never be + * able to change the parent URL) and allow-modals (alert/confirm/prompt are UX + * traps). Secure mode additionally OMITS allow-same-origin (forcing an opaque + * origin so the package cannot reach Moodle's DOM/cookies/session) and + * allow-popups-to-escape-sandbox (an escaped popup would reopen at Moodle's real + * origin without the sandbox). allow-scripts/allow-popups/allow-forms are kept + * in both modes because eXeLearning v4 iDevices need jQuery + scripts, popups + * (interactive-video, hidden-image) and forms (quick-questions, form, + * scrambled-list). See research ADR DEC-0059 / DEC-0019 / AN-008. + * + * @param string $mode self::MODE_SECURE or self::MODE_LEGACY. + * @return string Space-separated sandbox token list. + */ + public static function sandbox_tokens(string $mode): string { + if ($mode === self::MODE_LEGACY) { + return 'allow-scripts allow-same-origin allow-popups allow-forms allow-popups-to-escape-sandbox'; + } + return 'allow-scripts allow-popups allow-forms'; + } +} diff --git a/lang/en/exelearning.php b/lang/en/exelearning.php index db9195b..c4f7443 100644 --- a/lang/en/exelearning.php +++ b/lang/en/exelearning.php @@ -245,6 +245,10 @@ $string['gradepass_help'] = 'The minimum overall grade required to pass. When the "Require passing grade" completion condition is enabled, the activity is marked complete (SCORM-style) once the student reaches this grade. Leave at 0 to disable pass-based completion.'; $string['gradesetchangedwarning'] = 'The gradable content of this activity changed and some students already have attempts. Existing grades are kept as they were and are not recalculated against the new content. If the changes make those grades misleading, delete the affected attempts to recalculate them.'; $string['gradingheading'] = 'Grading'; +$string['iframemode'] = 'Package iframe security mode'; +$string['iframemode_desc'] = 'Controls how the eXeLearning package is embedded in the activity page. "Secure" (recommended) runs the package in a sandboxed, opaque-origin iframe so its JavaScript cannot read or change the surrounding Moodle page, its cookies or the session; SCORM scoring is relayed to Moodle through a validated postMessage channel. "Legacy" keeps the previous same-origin behaviour and should only be used if a specific package misbehaves under the secure mode (for example because it relies on browser storage that is not available to a sandboxed iframe).'; +$string['iframemode_legacy'] = 'Legacy (same-origin)'; +$string['iframemode_secure'] = 'Secure (sandboxed, isolated)'; $string['installstale'] = 'Installation may have failed. Please try again.'; $string['intro'] = 'Description'; $string['invalidaction'] = 'Invalid action: {$a}'; diff --git a/settings.php b/settings.php index 2bd1682..cc0ccb9 100644 --- a/settings.php +++ b/settings.php @@ -54,6 +54,25 @@ } if ($ADMIN->fulltree) { + // Package iframe security mode (DEC-0059): isolate the arbitrary author HTML/JS + // of an .elpx package in a sandboxed, opaque-origin iframe and relay SCORM + // scoring over a validated postMessage bridge. Defaults to the secure mode; + // 'legacy' restores the previous same-origin behaviour as a compatibility + // fallback. The default is also applied by player_iframe::resolve_mode() so an + // unset/invalid value never weakens isolation. + $settings->add(new admin_setting_configselect( + 'mod_exelearning/iframemode', + get_string('iframemode', 'mod_exelearning'), + get_string('iframemode_desc', 'mod_exelearning'), + \mod_exelearning\local\ui\player_iframe::MODE_SECURE, + [ + \mod_exelearning\local\ui\player_iframe::MODE_SECURE => + get_string('iframemode_secure', 'mod_exelearning'), + \mod_exelearning\local\ui\player_iframe::MODE_LEGACY => + get_string('iframemode_legacy', 'mod_exelearning'), + ] + )); + // Embedded editor management (install / update / repair / uninstall). $settings->add(new admin_setting_heading( 'mod_exelearning/embeddededitorheading', From 9c862dcddb45906c0217134ef4843fa072f769c8 Mon Sep 17 00:00:00 2001 From: erseco Date: Sat, 13 Jun 2026 09:12:23 +0100 Subject: [PATCH 02/35] feat(scorm): add transport option to scorm_tracker for the bridge --- js/scorm_tracker.js | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/js/scorm_tracker.js b/js/scorm_tracker.js index 934e567..40b97f3 100644 --- a/js/scorm_tracker.js +++ b/js/scorm_tracker.js @@ -133,6 +133,12 @@ * - cmid, trackurl, session: identity and endpoint. * - getScoringDocument(): returns the iframe content document (default: reads * #exelearningobject) for objectid resolution. + * - transport(data, sync): optional sink for the buffered scores. When provided + * it REPLACES the direct XHR (secure mode: js/scorm_bridge_shim.js posts the + * data to the Moodle parent, which owns the authenticated request). It gets + * {cmi, itemscores} and a sync flag, and returns false to signal failure + * (keeps the buffer dirty for retry). When absent, the XHR path below runs + * (legacy same-origin mode and the unit tests). * - xhrFactory(): returns an XMLHttpRequest-like object (default: real XHR). * - setTimeout / clearTimeout: timer functions (default: globals). * - bindUnload: wire a beforeunload synchronous flush (default: true in a browser). @@ -148,6 +154,7 @@ var setTimeoutFn = config.setTimeout || (typeof setTimeout !== 'undefined' ? setTimeout : null); var clearTimeoutFn = config.clearTimeout || (typeof clearTimeout !== 'undefined' ? clearTimeout : null); var xhrFactory = config.xhrFactory || function () { return new XMLHttpRequest(); }; + var transport = config.transport || null; var getScoringDocument = config.getScoringDocument || function () { var fr = (typeof document !== 'undefined') && document.getElementById('exelearningobject'); return fr && fr.contentDocument; @@ -161,6 +168,19 @@ function send(sync) { if (!dirty) { return true; } var snapshot = JSON.stringify(cmi); + // Bridge transport (secure mode): hand the buffered CMI + per-iDevice + // scores to the injected sink instead of doing the XHR here. The sink + // (js/scorm_bridge_shim.js) posts them to the Moodle parent, which owns + // the authenticated track.php request, retry and the pagehide beacon. + // Fire-and-forget: clear dirty once the message leaves; a thrown/false + // result keeps it dirty so the next autocommit re-sends it. + if (transport) { + try { + var accepted = transport({ cmi: cmi, itemscores: itemScores }, sync === true); + if (accepted !== false) { dirty = false; return true; } + return false; + } catch (te) { errCode = '101'; return false; } + } var payload = buildPayload(cmid, session, cmi, itemScores); try { var xhr = xhrFactory(); From 5a4b2209960d4e9958af51c14e9457e3f7b806e5 Mon Sep 17 00:00:00 2001 From: erseco Date: Sat, 13 Jun 2026 09:12:23 +0100 Subject: [PATCH 03/35] feat(scorm): add in-iframe SCORM bridge shim with storage polyfill --- js/scorm_bridge_shim.js | 245 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 245 insertions(+) create mode 100644 js/scorm_bridge_shim.js diff --git a/js/scorm_bridge_shim.js b/js/scorm_bridge_shim.js new file mode 100644 index 0000000..e14abe7 --- /dev/null +++ b/js/scorm_bridge_shim.js @@ -0,0 +1,245 @@ +// This file is part of Moodle - http://moodle.org/ +// +// Moodle is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Moodle is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Moodle. If not, see . + +/** + * In-iframe SCORM bridge shim for the secure (opaque-origin) package mode. + * + * This script is baked into every extracted package (copied to libs/ and injected + * at the top of by \mod_exelearning\local\scorm\scorm_injector) and runs + * INSIDE the package iframe. It self-activates only when the iframe is a sandboxed, + * opaque-origin document (secure mode, view.php drops allow-same-origin); in the + * legacy same-origin mode it stays dormant so eXeLearning's pipwerks walks up to + * the window.API hosted by the Moodle parent, exactly as before. + * + * When active it: + * 1. Installs an in-memory localStorage/sessionStorage polyfill, because an + * opaque-origin document throws SecurityError on real web storage and several + * shipped engine scripts (libs/exe_atools, exe_export.js, the checklist iDevice, + * edicuatex) touch it. The polyfill keeps them working for the session. + * 2. Defines a local window.API (the SCORM 1.2 surface from js/scorm_tracker.js) + * whose buffered scores are posted to the Moodle parent over postMessage instead + * of being XHR'd here. The parent (js/scorm_bridge_relay.js) holds the sesskey + * and performs the authenticated track.php request; this iframe never sees it. + * 3. Performs a handshake: it announces 'ready', the parent replies with a nonce + + * the teacher-mode preference, and the shim hides eXeLearning's teacher toggle + * locally (the parent cannot reach this opaque document). + * + * Exposed two ways from a single body: window.exeScormBridgeShim (browser, with an + * auto-boot that is a no-op outside an opaque sandbox) and module.exports (Vitest). + * See research ADR DEC-0059. + */ +(function () { + 'use strict'; + + /** + * Build an in-memory Storage-like object (getItem/setItem/removeItem/clear/key + * + length), used to shadow the native web storage that throws in an opaque + * origin. Values are coerced to strings, matching the Storage contract. + * + * @returns {Object} A minimal in-memory Storage implementation. + */ + function createMemoryStorage() { + var store = {}; + var api = { + getItem: function (k) { + k = String(k); + return Object.prototype.hasOwnProperty.call(store, k) ? store[k] : null; + }, + setItem: function (k, v) { store[String(k)] = String(v); }, + removeItem: function (k) { delete store[String(k)]; }, + clear: function () { store = {}; }, + key: function (i) { + var keys = Object.keys(store); + return (i >= 0 && i < keys.length) ? keys[i] : null; + } + }; + Object.defineProperty(api, 'length', { get: function () { return Object.keys(store).length; } }); + return api; + } + + /** + * Detect whether the current window is a sandboxed, opaque-origin document + * (secure mode). Opaque origins serialize to the string "null"; as a secondary + * probe, real web storage access throws. Either signal means "activate". + * + * @param {Window} win The window to test (default: the global window). + * @returns {boolean} True when running in an opaque sandbox. + */ + function isSandboxedOpaque(win) { + win = win || (typeof window !== 'undefined' ? window : null); + if (!win) { return false; } + try { + if (win.origin === 'null' || (win.location && win.location.origin === 'null')) { return true; } + } catch (e) { return true; } + try { + var probe = '__exeprobe__'; + win.localStorage.setItem(probe, '1'); + win.localStorage.removeItem(probe); + } catch (e2) { return true; } + return false; + } + + /** + * Replace win.localStorage/win.sessionStorage with in-memory polyfills so package + * scripts that touch web storage do not throw in an opaque origin. Best effort: + * if the property cannot be redefined, content storage access may still throw, but + * grading (which never relies on web storage) is unaffected. + * + * @param {Window} win The window to patch. + */ + function installStoragePolyfill(win) { + var names = ['localStorage', 'sessionStorage']; + for (var i = 0; i < names.length; i++) { + var name = names[i]; + var mem = createMemoryStorage(); + try { + Object.defineProperty(win, name, { + configurable: true, + get: (function (m) { return function () { return m; }; })(mem) + }); + } catch (e) { + try { win[name] = mem; } catch (e2) { /* give up; see docblock. */ } + } + } + } + + /** + * Whether a postMessage payload is a recognised parent -> child control message. + * + * @param {*} data The event.data value. + * @returns {boolean} True for a {type:'scorm', action:'config'|'ack'} message. + */ + function isParentMessage(data) { + return !!data && data.type === 'scorm' && (data.action === 'config' || data.action === 'ack'); + } + + /** + * Wire the local window.API to the Moodle parent over postMessage. Requires + * win.exeScormTracker (js/scorm_tracker.js) to be loaded first. + * + * @param {Window} win The (opaque-origin) iframe window. + * @returns {Object|null} Handles for testing, or null if the tracker is missing. + */ + function activate(win) { + var tracker = win.exeScormTracker; + if (!tracker) { return null; } + + var parentwin = win.parent; + var nonce = null; + var ready = false; + var queue = []; + + function postToParent(msg) { + try { + if (parentwin && parentwin.postMessage) { parentwin.postMessage(msg, '*'); } + } catch (e) { /* ignore */ } + } + + // Bridge transport handed to the shared tracker: forward buffered scores to + // the parent. Identity (cmid, sesskey) lives in the parent; only the CMI + // buffer + per-iDevice scores cross the bridge, and track.php re-validates + // and clamps them server-side. Fire-and-forget; queued until the handshake + // delivers the nonce. + function transport(data) { + var msg = { + exelearningBridge: nonce, + type: 'scorm', + action: 'track', + cmi: data.cmi, + itemscores: data.itemscores + }; + if (ready) { postToParent(msg); } else { queue.push(msg); } + return true; + } + + var instance = tracker.createScormApi({ + transport: transport, + // Resolve per-iDevice objectids from THIS document (same frame), not the + // parent's iframe element. + getScoringDocument: function () { return win.document; }, + // No beforeunload flush here: postMessage is async and may not reach the + // parent during unload. The parent's pagehide sendBeacon (relay) is the + // reliable unload safety net instead. + bindUnload: false + }); + win.API = instance.api; + + function hideTeacherMode() { + try { + var d = win.document; + var st = d.createElement('style'); + st.textContent = '#teacher-mode-toggler-wrapper { visibility: hidden !important; }'; + (d.head || d.documentElement).appendChild(st); + } catch (e) { /* best effort */ } + } + + function onMessage(e) { + if (e.source !== parentwin) { return; } // Only trust the hosting Moodle frame. + var data = e.data; + if (!isParentMessage(data)) { return; } + if (data.action === 'config') { + nonce = data.nonce; + ready = true; + if (!data.teachermodevisible) { hideTeacherMode(); } + while (queue.length) { + var m = queue.shift(); + m.exelearningBridge = nonce; + postToParent(m); + } + } + } + + if (win.addEventListener) { win.addEventListener('message', onMessage, false); } + // Announce readiness; the parent replies with the nonce + teacher-mode flag. + postToParent({ exelearningBridge: null, type: 'scorm', action: 'ready' }); + + return { + api: instance.api, + transport: transport, + onMessage: onMessage, + hideTeacherMode: hideTeacherMode + }; + } + + /** + * Boot the shim: activate only inside an opaque sandbox. No-op otherwise (legacy + * same-origin mode, or any non-sandboxed context such as the test runner). + * + * @param {Window} win The window to boot in (default: the global window). + * @returns {Object|null} The activate() handles when activated, else null. + */ + function boot(win) { + win = win || (typeof window !== 'undefined' ? window : null); + if (!win || !isSandboxedOpaque(win)) { return null; } + installStoragePolyfill(win); + return activate(win); + } + + var exp = { + createMemoryStorage: createMemoryStorage, + isSandboxedOpaque: isSandboxedOpaque, + installStoragePolyfill: installStoragePolyfill, + isParentMessage: isParentMessage, + activate: activate, + boot: boot + }; + // Test runner (Vitest/Node) consumes module.exports. + if (typeof module !== 'undefined' && module.exports) { module.exports = exp; } + // Browser: expose for inspection and auto-boot (no-op outside an opaque sandbox). + if (typeof window !== 'undefined') { + window.exeScormBridgeShim = exp; + boot(window); + } +})(); From f91036bea53a906abc4ce8dfdd44aa063d136081 Mon Sep 17 00:00:00 2001 From: erseco Date: Sat, 13 Jun 2026 09:12:23 +0100 Subject: [PATCH 04/35] feat(security): add parent-side postMessage relay; opaque-origin iframe in view.php --- js/scorm_bridge_relay.js | 191 +++++++++++++++++++++++++++++++++++++++ view.php | 107 ++++++++++++++-------- 2 files changed, 259 insertions(+), 39 deletions(-) create mode 100644 js/scorm_bridge_relay.js diff --git a/js/scorm_bridge_relay.js b/js/scorm_bridge_relay.js new file mode 100644 index 0000000..e1ffc60 --- /dev/null +++ b/js/scorm_bridge_relay.js @@ -0,0 +1,191 @@ +// This file is part of Moodle - http://moodle.org/ +// +// Moodle is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Moodle is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Moodle. If not, see . + +/** + * Parent-side SCORM bridge relay for the secure (opaque-origin) package mode. + * + * Injected inline by view.php (secure mode only) so its message listener is in place + * before the package iframe loads. It is the trusted half of the bridge: the iframe + * runs in an opaque origin and CANNOT reach this page, so the only thing it can do is + * postMessage buffered SCORM scores here. This relay validates every message and, for + * accepted ones, performs the authenticated track.php request (keeping the sesskey on + * this trusted side) plus a sendBeacon flush on pagehide. + * + * Validation (defence in depth — track.php re-validates and clamps server-side): + * - event.source === iframe.contentWindow (window identity, the primary anchor: + * no other window can forge it, and an opaque origin has no useful event.origin); + * - type === 'scorm' and action in the closed list {ready, track}; + * - a per-view nonce on 'track' messages; + * - the payload shape (cmi is an object). + * Unknown or invalid messages are ignored silently. + * + * Exposed two ways from a single body: window.exeScormBridge (browser bootstrap) and + * module.exports (Vitest). See research ADR DEC-0059. + */ +(function () { + 'use strict'; + + /** + * Whether a payload is a child -> parent score message (shape only). + * + * @param {*} data The event.data value. + * @returns {boolean} True for {type:'scorm', action:'track', cmi:{...}}. + */ + function isTrackMessage(data) { + return !!data && data.type === 'scorm' && data.action === 'track' + && !!data.cmi && typeof data.cmi === 'object'; + } + + /** + * Whether a payload is the child's readiness announcement. + * + * @param {*} data The event.data value. + * @returns {boolean} True for {type:'scorm', action:'ready'}. + */ + function isReadyMessage(data) { + return !!data && data.type === 'scorm' && data.action === 'ready'; + } + + /** + * Whether a 'track' message should be accepted: correct shape AND matching nonce. + * Pure, so it can be unit-tested without a DOM. The caller is still responsible + * for the window-identity check (which cannot be expressed on data alone). + * + * @param {*} data The event.data value. + * @param {string} expectednonce The per-view nonce handed to the iframe. + * @returns {boolean} True when the message is a valid, authenticated track message. + */ + function acceptTrack(data, expectednonce) { + return isTrackMessage(data) && data.exelearningBridge === expectednonce; + } + + /** + * Create a relay bound to a config + (injectable) environment. + * + * @param {Object} config {iframeid, cmid, trackurl, session, nonce, teachermodevisible}. + * @param {Object} [deps] {document, window, fetch, sendBeacon} for testing. + * @returns {Object} {init, onMessage, flushBeacon, postTrack, acceptTrack}. + */ + function createRelay(config, deps) { + config = config || {}; + deps = deps || {}; + var doc = deps.document || (typeof document !== 'undefined' ? document : null); + var win = deps.window || (typeof window !== 'undefined' ? window : null); + var fetchImpl = deps.fetch || (win && win.fetch ? win.fetch.bind(win) : null); + var beacon = deps.sendBeacon + || (win && win.navigator && win.navigator.sendBeacon + ? win.navigator.sendBeacon.bind(win.navigator) : null); + + var iframeid = config.iframeid; + var trackurl = config.trackurl; + var cmid = config.cmid; + var session = config.session; + var nonce = config.nonce; + var teachermodevisible = config.teachermodevisible ? 1 : 0; + var latest = null; + + function iframe() { return doc ? doc.getElementById(iframeid) : null; } + + function buildBody(cmi, itemscores) { + // Mirror the legacy track.php payload, but identity (cmid in the query, + // sesskey, mode) lives on this trusted parent — only the CMI buffer and + // per-iDevice scores come from the iframe. + return JSON.stringify({ + id: cmid, + session: session, + cmi: cmi, + itemscores: itemscores || {} + }); + } + + function postTrack(cmi, itemscores) { + var body = buildBody(cmi, itemscores); + latest = body; + if (!fetchImpl) { return; } + try { + fetchImpl(trackurl, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: body, + credentials: 'same-origin', + keepalive: true + }).catch(function () { /* parent retries on the next commit / pagehide beacon. */ }); + } catch (e) { /* ignore */ } + } + + function flushBeacon() { + if (!latest || !beacon) { return; } + try { beacon(trackurl, new Blob([latest], { type: 'application/json' })); } catch (e) { /* ignore */ } + } + + function onMessage(e) { + var fr = iframe(); + if (!fr || e.source !== fr.contentWindow) { return; } // Window identity (primary anchor). + var data = e.data; + if (isReadyMessage(data)) { + try { + fr.contentWindow.postMessage({ + type: 'scorm', + action: 'config', + nonce: nonce, + teachermodevisible: teachermodevisible + }, '*'); + } catch (e2) { /* ignore */ } + return; + } + if (!acceptTrack(data, nonce)) { return; } // type + action + nonce + shape. + postTrack(data.cmi, data.itemscores); + } + + function init() { + if (win && win.addEventListener) { + win.addEventListener('message', onMessage, false); + win.addEventListener('pagehide', flushBeacon, false); + } + } + + return { + init: init, + onMessage: onMessage, + flushBeacon: flushBeacon, + postTrack: postTrack, + acceptTrack: acceptTrack + }; + } + + /** + * Bootstrap: create a relay from config and start listening. + * + * @param {Object} config See createRelay. + * @returns {Object} The relay instance. + */ + function init(config) { + var relay = createRelay(config); + relay.init(); + return relay; + } + + var exp = { + isTrackMessage: isTrackMessage, + isReadyMessage: isReadyMessage, + acceptTrack: acceptTrack, + createRelay: createRelay, + init: init + }; + // Test runner (Vitest/Node) consumes module.exports. + if (typeof module !== 'undefined' && module.exports) { module.exports = exp; } + // Browser bootstrap (view.php) consumes window.exeScormBridge. + if (typeof window !== 'undefined') { window.exeScormBridge = exp; } +})(); diff --git a/view.php b/view.php index 0f63719..7b23dd2 100644 --- a/view.php +++ b/view.php @@ -378,33 +378,66 @@ ); } } - // SCORM 1.2 shim: injects window.API into the parent window of the iframe. - // pipwerks SCORM (used by eXeLearning v4 iDevices) calls `findAPI()`, - // walking `window.parent` looking for an `API` object with `LMSInitialize`. - // If not found, the iDevice shows "This page is not part of a SCORM package". - // Minimal viable implementation: buffers CMI pairs and sends them to - // track.php on LMSCommit/LMSFinish. + // SCORM 1.2 client. eXeLearning v4 iDevices use pipwerks SCORM, which calls + // findAPI() looking for an `API` object with LMSInitialize. How that API is + // provided depends on the configured iframe security mode (DEC-0059). In secure + // mode (the default) the package runs in an opaque-origin sandboxed iframe and + // CANNOT reach this page: window.API lives INSIDE the iframe (baked bridge shim, + // libs/exe_scorm_bridge.js) and scoring is relayed here over a validated postMessage + // channel, with this parent only forwarding it to track.php so the sesskey stays on + // the trusted side. In legacy mode the package is same-origin, window.API is injected + // here in the parent, and the iframe's pipwerks walks window.parent to find it. $trackurl = (new moodle_url( '/mod/exelearning/track.php', ['id' => $cm->id, 'sesskey' => sesskey(), 'mode' => $mode] ))->out(false); - // The tracker logic is a single source of truth in js/scorm_tracker.js, also - // unit-tested with Vitest (tests/js/scorm_tracker.test.js). It is injected inline - // (not as an AMD module) so window.API is defined synchronously before the package - // iframe's pipwerks findAPI() runs — an async AMD load would race the SCO and break - // grading. Config (cmid, track URL, per-page attempt token) is passed as JSON to the - // createScormApi() factory instead of string-substituted placeholders. - $scormcfg = json_encode( - [ - 'cmid' => (int) $cm->id, - 'trackurl' => $trackurl, - 'session' => random_string(20), - ], - JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT - ); - $trackerjs = file_get_contents(__DIR__ . '/js/scorm_tracker.js'); - $bootjs = "\n(function () { window.API = window.exeScormTracker.createScormApi($scormcfg).api; })();"; - echo html_writer::tag('script', $trackerjs . $bootjs); + + $iframemode = \mod_exelearning\local\ui\player_iframe::resolve_mode(); + $securemode = ($iframemode === \mod_exelearning\local\ui\player_iframe::MODE_SECURE); + + if ($securemode) { + // Parent-side bridge relay (js/scorm_bridge_relay.js, also unit-tested with + // Vitest). Injected inline so its message listener is installed before the + // iframe loads and emits its first message. It validates each message by + // window identity (event.source === iframe.contentWindow), a closed action + // list and a per-view nonce, then performs the authenticated track.php POST + // (and a sendBeacon flush on pagehide). The nonce, the per-page attempt token + // and the teacher-mode preference are handed to the in-iframe shim during the + // handshake; the sesskey is NEVER sent across the bridge. + $relaycfg = json_encode( + [ + 'iframeid' => 'exelearningobject', + 'cmid' => (int) $cm->id, + 'trackurl' => $trackurl, + 'session' => random_string(20), + 'nonce' => random_string(32), + 'teachermodevisible' => (int) !empty($exelearning->teachermodevisible), + ], + JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT + ); + $relayjs = file_get_contents(__DIR__ . '/js/scorm_bridge_relay.js'); + $bootjs = "\n(function () { window.exeScormBridge.init($relaycfg); })();"; + echo html_writer::tag('script', $relayjs . $bootjs); + } else { + // Legacy same-origin: host window.API in this parent window. The tracker logic + // is the single source of truth in js/scorm_tracker.js, also unit-tested with + // Vitest (tests/js/scorm_tracker.test.js). It is injected inline (not as an AMD + // module) so window.API is defined synchronously before the package iframe's + // pipwerks findAPI() runs — an async AMD load would race the SCO and break + // grading. Config (cmid, track URL, per-page attempt token) is passed as JSON + // to the createScormApi() factory instead of string-substituted placeholders. + $scormcfg = json_encode( + [ + 'cmid' => (int) $cm->id, + 'trackurl' => $trackurl, + 'session' => random_string(20), + ], + JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT + ); + $trackerjs = file_get_contents(__DIR__ . '/js/scorm_tracker.js'); + $bootjs = "\n(function () { window.API = window.exeScormTracker.createScormApi($scormcfg).api; })();"; + echo html_writer::tag('script', $trackerjs . $bootjs); + } // Fullscreen control (issue #13 #6, DEC-0024): a right-aligned button above the // player. The iframe already advertises allow="fullscreen"; amd/src/fullscreen.js @@ -426,16 +459,11 @@ ); $PAGE->requires->js_call_amd('mod_exelearning/fullscreen', 'init', ['exelearningobject']); - // Package iframe. Sandbox policy documented in AN-008: - // allow-scripts: eXeLearning v4 uses jQuery + iDevice JS. - // allow-same-origin: relative paths to pluginfile.php/.../content// - // and future postMessage to the xAPI endpoint. - // allow-popups: interactive-video, hidden-image, etc. - // allow-forms: quick-questions, form, scrambled-list, etc. - // allow-popups-to-escape-sandbox: popups load without restrictions. - // Explicitly BLOCKED (not included): - // allow-top-navigation: a malicious package must not change the parent URL. - // allow-modals: no alert/confirm/prompt, they are UX interruptions. + // Package iframe. The sandbox token list depends on the configured iframe + // security mode (\mod_exelearning\local\ui\player_iframe, DEC-0059). Both modes + // omit allow-top-navigation (a package must not change the parent URL) and + // allow-modals; secure mode also omits allow-same-origin (opaque origin, so the + // package cannot reach this page) and allow-popups-to-escape-sandbox. echo html_writer::tag('iframe', '', [ 'src' => $iframeurl->out(false), 'name' => 'exelearningobject', @@ -444,15 +472,16 @@ 'width' => '100%', 'height' => '650', 'allow' => 'fullscreen', - 'sandbox' => 'allow-scripts allow-same-origin allow-popups allow-forms allow-popups-to-escape-sandbox', + 'sandbox' => \mod_exelearning\local\ui\player_iframe::sandbox_tokens($iframemode), 'style' => 'border: 1px solid var(--bs-border-color, #dee2e6); border-radius: .5rem;', ]); - // Hide eXeLearning's teacher-mode toggle inside the package (mod_exeweb - // parity): when teachermodevisible=0, inject CSS into the iframe content to - // hide #teacher-mode-toggler-wrapper. The iframe is same-origin (served via - // pluginfile.php) so the parent can reach its content document. - if (empty($exelearning->teachermodevisible)) { + // Hide eXeLearning's teacher-mode toggle inside the package (mod_exeweb parity) + // when teachermodevisible=0. In legacy mode the parent injects CSS into the + // same-origin iframe document. In secure mode the iframe is opaque-origin and + // unreachable from here, so the in-iframe shim hides it locally using the + // teachermodevisible flag passed over the bridge handshake (see $relaycfg above). + if (empty($exelearning->teachermodevisible) && !$securemode) { exelearning_require_teacher_mode_hider('exelearningobject'); } } From 9da6935fb9c46ee961eb634c1522db1a607da685 Mon Sep 17 00:00:00 2001 From: erseco Date: Sat, 13 Jun 2026 09:12:23 +0100 Subject: [PATCH 05/35] feat(scorm): inject and ship the bridge client into the package --- classes/local/package_manager.php | 38 +++++++++++++------- classes/local/scorm/scorm_injector.php | 49 ++++++++++++++++++++++---- 2 files changed, 69 insertions(+), 18 deletions(-) diff --git a/classes/local/package_manager.php b/classes/local/package_manager.php index e1e077a..1ee2b86 100644 --- a/classes/local/package_manager.php +++ b/classes/local/package_manager.php @@ -239,25 +239,39 @@ public static function extract_stored(int $contextid, int $revision): void { 1 ); - // 5) If the package (web export) does not include libs/SCORM_API_wrapper.js, - // inject it from the plugin's assets/ directory. eXeLearning v4 only bundles - // this wrapper in the SCORM export; without it, gradable iDevices display - // "this page is not part of a SCORM package". - foreach (['SCORM_API_wrapper.js', 'SCOFunctions.js'] as $shimname) { + // 5) Ensure the SCORM client assets live under libs/ of the extracted package. + // The vendored pipwerks wrapper + SCOFunctions are copied only when the export + // does not already bundle them: eXeLearning v4 bundles them in the SCORM export + // but not in the web/elpx export, and without them gradable iDevices show "this + // page is not part of a SCORM package". A bundled copy is never overwritten + // ($refresh = false). The bridge client (scorm_tracker + exe_scorm_bridge) powers + // the secure opaque-origin iframe mode (DEC-0059); it runs INSIDE the iframe so it + // must be served from the package, and being plugin-owned it is refreshed on every + // extract so a shim update reaches existing packages ($refresh = true). + $clientassets = [ + ['SCORM_API_wrapper.js', __DIR__ . '/../../assets/scorm/SCORM_API_wrapper.js', false], + ['SCOFunctions.js', __DIR__ . '/../../assets/scorm/SCOFunctions.js', false], + ['scorm_tracker.js', __DIR__ . '/../../js/scorm_tracker.js', true], + ['exe_scorm_bridge.js', __DIR__ . '/../../js/scorm_bridge_shim.js', true], + ]; + foreach ($clientassets as $asset) { + [$destname, $assetpath, $refresh] = $asset; + if (!is_file($assetpath)) { + continue; + } $present = $fs->get_file( $context->id, 'mod_exelearning', 'content', (int) $data->revision, '/libs/', - $shimname + $destname ); if ($present) { - continue; - } - $assetpath = __DIR__ . '/../../assets/scorm/' . $shimname; - if (!is_file($assetpath)) { - continue; + if (!$refresh) { + continue; + } + $present->delete(); } $fs->create_file_from_pathname([ 'contextid' => $context->id, @@ -265,7 +279,7 @@ public static function extract_stored(int $contextid, int $revision): void { 'filearea' => 'content', 'itemid' => (int) $data->revision, 'filepath' => '/libs/', - 'filename' => $shimname, + 'filename' => $destname, ], $assetpath); } diff --git a/classes/local/scorm/scorm_injector.php b/classes/local/scorm/scorm_injector.php index 7a477ce..6cc5400 100644 --- a/classes/local/scorm/scorm_injector.php +++ b/classes/local/scorm/scorm_injector.php @@ -33,15 +33,24 @@ */ final class scorm_injector { /** - * Injects SCORM wrapper script tags into the of index.html and all + * Injects the SCORM client script tags into the of index.html and all * html/.html pages of the extracted package. * + * Two independent, idempotent blocks are injected: + * - The bridge client (libs/scorm_tracker.js + libs/exe_scorm_bridge.js) at the + * TOP of , so its in-memory storage polyfill and local window.API are in + * place before any package script runs. It self-activates only in the secure + * opaque-origin iframe mode and is dormant otherwise (DEC-0059). + * - The pipwerks SCORM wrapper (libs/SCORM_API_wrapper.js + libs/SCOFunctions.js) + * plus an init kick, just before , used by both iframe modes. + * * @param int $contextid * @param int $revision */ public static function inject(int $contextid, int $revision): void { $fs = get_file_storage(); $marker = ''; + $bridgemarker = ''; // After loading the wrapper, force `pipwerks.SCORM.init()` so that // connection.isActive=true and subsequent `set()` calls DO reach // window.parent.API.LMSSetValue. eXeLearning only invokes init() in the @@ -65,6 +74,14 @@ public static function inject(int $contextid, int $revision): void { "\n " . "\n " . $initscript; + // Bridge client. scorm_tracker.js must load before exe_scorm_bridge.js (the + // shim calls window.exeScormTracker.createScormApi). + $bridge = $bridgemarker . + "\n " . + "\n \n"; + $bridgehtml = $bridgemarker . + "\n " . + "\n \n"; // Iterate over all HTML files in the filearea. $files = $fs->get_area_files( @@ -84,14 +101,34 @@ public static function inject(int $contextid, int $revision): void { continue; } $html = $file->get_content(); - if ($html === '' || strpos($html, $marker) !== false) { + if ($html === '') { continue; } $path = $file->get_filepath(); - $payload = ($path === '/') ? $tags : $tagshtml; - // Insert just before (case-insensitive). - $newhtml = preg_replace('~~i', $payload . '', $html, 1); - if ($newhtml === null || $newhtml === $html) { + $newhtml = $html; + $changed = false; + + // Bridge client at the top of (case-insensitive, first match). + if (strpos($newhtml, $bridgemarker) === false) { + $bridgepayload = ($path === '/') ? $bridge : $bridgehtml; + $replaced = preg_replace('~(]*>)~i', '${1}' . $bridgepayload, $newhtml, 1); + if ($replaced !== null && $replaced !== $newhtml) { + $newhtml = $replaced; + $changed = true; + } + } + + // SCORM wrapper just before (case-insensitive, first match). + if (strpos($newhtml, $marker) === false) { + $payload = ($path === '/') ? $tags : $tagshtml; + $replaced = preg_replace('~~i', $payload . '', $newhtml, 1); + if ($replaced !== null && $replaced !== $newhtml) { + $newhtml = $replaced; + $changed = true; + } + } + + if (!$changed) { continue; } // Replace content in the filearea: delete and recreate. From 70deb3da98f41cef0f8b1f6f467057f88ab40495 Mon Sep 17 00:00:00 2001 From: erseco Date: Sat, 13 Jun 2026 09:12:23 +0100 Subject: [PATCH 06/35] test: PHPUnit iframe-mode + Vitest bridge/relay/storage tests --- tests/js/scorm_bridge.test.js | 260 ++++++++++++++++++++++++++++++++++ tests/player_iframe_test.php | 99 +++++++++++++ 2 files changed, 359 insertions(+) create mode 100644 tests/js/scorm_bridge.test.js create mode 100644 tests/player_iframe_test.php diff --git a/tests/js/scorm_bridge.test.js b/tests/js/scorm_bridge.test.js new file mode 100644 index 0000000..ed5015f --- /dev/null +++ b/tests/js/scorm_bridge.test.js @@ -0,0 +1,260 @@ +// This file is part of Moodle - http://moodle.org/ +// +// Moodle is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Moodle is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Moodle. If not, see . + +// Side-effect imports: each module exposes its API on a window.* global (and on +// module.exports). scorm_tracker must load before the shim (the shim calls +// window.exeScormTracker). globals (describe/it/expect/vi) come from vitest.config.mjs. +import '../../js/scorm_tracker.js'; +import '../../js/scorm_bridge_shim.js'; +import '../../js/scorm_bridge_relay.js'; + +const shim = window.exeScormBridgeShim; +const relay = window.exeScormBridge; + +/** + * Build a fake iframe window for the shim: captures messages posted to the parent + * and the registered 'message' listener so a test can drive the handshake. Uses the + * real (happy-dom) document so the shared tracker's objectid resolution works. + */ +function makeFakeWin(overrides = {}) { + const postedToParent = []; + const listeners = {}; + const win = { + exeScormTracker: window.exeScormTracker, + parent: { postMessage: (msg) => postedToParent.push(msg) }, + origin: 'null', + document, + addEventListener: (type, fn) => { (listeners[type] = listeners[type] || []).push(fn); }, + postedToParent, + listeners, + ...overrides, + }; + return win; +} + +/** Deliver a message to the shim's registered onMessage listener. */ +function deliver(win, event) { + (win.listeners.message || []).forEach((fn) => fn(event)); +} + +describe('shim createMemoryStorage', () => { + it('behaves like a string-coercing in-memory Storage', () => { + const s = shim.createMemoryStorage(); + expect(s.getItem('missing')).toBeNull(); + s.setItem('a', 1); + expect(s.getItem('a')).toBe('1'); // coerced to string + expect(s.length).toBe(1); + expect(s.key(0)).toBe('a'); + s.removeItem('a'); + expect(s.getItem('a')).toBeNull(); + expect(s.length).toBe(0); + s.setItem('b', 'x'); + s.clear(); + expect(s.length).toBe(0); + }); +}); + +describe('shim isSandboxedOpaque', () => { + it('is true when the origin serializes to "null"', () => { + expect(shim.isSandboxedOpaque({ origin: 'null' })).toBe(true); + }); + + it('is true when web storage access throws (opaque origin)', () => { + const win = { + origin: 'https://moodle.test', + get localStorage() { throw new Error('SecurityError'); }, + }; + expect(shim.isSandboxedOpaque(win)).toBe(true); + }); + + it('is false for a same-origin window with working storage', () => { + const win = { origin: 'https://moodle.test', localStorage: shim.createMemoryStorage() }; + expect(shim.isSandboxedOpaque(win)).toBe(false); + }); +}); + +describe('shim installStoragePolyfill', () => { + it('replaces storage with in-memory implementations', () => { + const win = {}; + shim.installStoragePolyfill(win); + win.localStorage.setItem('k', 'v'); + expect(win.localStorage.getItem('k')).toBe('v'); + win.sessionStorage.setItem('s', '1'); + expect(win.sessionStorage.getItem('s')).toBe('1'); + }); +}); + +describe('shim isParentMessage', () => { + it('accepts config/ack and rejects everything else', () => { + expect(shim.isParentMessage({ type: 'scorm', action: 'config' })).toBe(true); + expect(shim.isParentMessage({ type: 'scorm', action: 'ack' })).toBe(true); + expect(shim.isParentMessage({ type: 'scorm', action: 'track' })).toBe(false); + expect(shim.isParentMessage({ type: 'other', action: 'config' })).toBe(false); + expect(shim.isParentMessage(null)).toBe(false); + }); +}); + +describe('shim activate (handshake + transport)', () => { + it('announces ready, then defines window.API', () => { + const win = makeFakeWin(); + const handles = shim.activate(win); + expect(handles).not.toBeNull(); + expect(win.API).toBe(handles.api); + expect(win.postedToParent[0]).toEqual({ exelearningBridge: null, type: 'scorm', action: 'ready' }); + }); + + it('queues scores until config arrives, then flushes them stamped with the nonce', () => { + const win = makeFakeWin(); + shim.activate(win); + // Score before the handshake completes: must be queued, not posted yet. + win.API.LMSSetValue('cmi.core.score.raw', '80'); + win.API.LMSCommit(); + expect(win.postedToParent.filter((m) => m.action === 'track')).toHaveLength(0); + // Parent replies with the nonce -> queued track message is flushed. + deliver(win, { source: win.parent, data: { type: 'scorm', action: 'config', nonce: 'N1', teachermodevisible: 1 } }); + const tracks = win.postedToParent.filter((m) => m.action === 'track'); + expect(tracks).toHaveLength(1); + expect(tracks[0].exelearningBridge).toBe('N1'); + expect(tracks[0].cmi['cmi.core.score.raw']).toBe('80'); + }); + + it('posts scores immediately once ready', () => { + const win = makeFakeWin(); + shim.activate(win); + deliver(win, { source: win.parent, data: { type: 'scorm', action: 'config', nonce: 'N2', teachermodevisible: 1 } }); + win.API.LMSSetValue('cmi.core.lesson_status', 'completed'); + win.API.LMSCommit(); + const tracks = win.postedToParent.filter((m) => m.action === 'track'); + expect(tracks).toHaveLength(1); + expect(tracks[0].exelearningBridge).toBe('N2'); + expect(tracks[0].cmi['cmi.core.lesson_status']).toBe('completed'); + }); + + it('ignores messages whose source is not the parent window', () => { + const win = makeFakeWin(); + shim.activate(win); + win.API.LMSSetValue('cmi.core.score.raw', '50'); + win.API.LMSCommit(); + // A config from a foreign window must not unblock the queue. + deliver(win, { source: { other: true }, data: { type: 'scorm', action: 'config', nonce: 'EVIL', teachermodevisible: 1 } }); + expect(win.postedToParent.filter((m) => m.action === 'track')).toHaveLength(0); + }); + + it('hides the teacher-mode toggle when teachermodevisible is falsy', () => { + const win = makeFakeWin(); + shim.activate(win); + const before = document.querySelectorAll('style').length; + deliver(win, { source: win.parent, data: { type: 'scorm', action: 'config', nonce: 'N3', teachermodevisible: 0 } }); + expect(document.querySelectorAll('style').length).toBe(before + 1); + }); +}); + +describe('shim boot', () => { + it('activates in an opaque sandbox', () => { + const win = makeFakeWin({ origin: 'null' }); + const handles = shim.boot(win); + expect(handles).not.toBeNull(); + expect(win.API).toBeDefined(); + // Storage was polyfilled. + win.localStorage.setItem('k', 'v'); + expect(win.localStorage.getItem('k')).toBe('v'); + }); + + it('stays dormant (no window.API) outside an opaque sandbox', () => { + const win = makeFakeWin({ origin: 'https://moodle.test', localStorage: shim.createMemoryStorage(), API: undefined }); + const handles = shim.boot(win); + expect(handles).toBeNull(); + expect(win.API).toBeUndefined(); + }); +}); + +describe('relay pure validators', () => { + it('isTrackMessage requires the scorm/track shape with a cmi object', () => { + expect(relay.isTrackMessage({ type: 'scorm', action: 'track', cmi: {} })).toBe(true); + expect(relay.isTrackMessage({ type: 'scorm', action: 'track' })).toBe(false); + expect(relay.isTrackMessage({ type: 'scorm', action: 'ready' })).toBe(false); + expect(relay.isTrackMessage(null)).toBe(false); + }); + + it('isReadyMessage matches only the readiness announcement', () => { + expect(relay.isReadyMessage({ type: 'scorm', action: 'ready' })).toBe(true); + expect(relay.isReadyMessage({ type: 'scorm', action: 'track', cmi: {} })).toBe(false); + }); + + it('acceptTrack requires both a valid shape and the matching nonce', () => { + const msg = { type: 'scorm', action: 'track', cmi: {}, exelearningBridge: 'N' }; + expect(relay.acceptTrack(msg, 'N')).toBe(true); + expect(relay.acceptTrack(msg, 'OTHER')).toBe(false); + expect(relay.acceptTrack({ type: 'scorm', action: 'track', cmi: {}, exelearningBridge: undefined }, 'N')).toBe(false); + }); +}); + +describe('relay createRelay (message handling)', () => { + function setup() { + const cw = { postMessage: vi.fn() }; + const iframe = { contentWindow: cw }; + const doc = { getElementById: (id) => (id === 'exelearningobject' ? iframe : null) }; + const fetchCalls = []; + const fetchImpl = (url, opts) => { fetchCalls.push({ url, opts }); return { catch: () => {} }; }; + const beaconCalls = []; + const sendBeacon = (url, blob) => { beaconCalls.push({ url, blob }); return true; }; + const r = relay.createRelay( + { iframeid: 'exelearningobject', cmid: 42, trackurl: '/track.php?id=42', session: 'tok', nonce: 'N', teachermodevisible: 0 }, + { document: doc, window: { addEventListener: () => {} }, fetch: fetchImpl, sendBeacon } + ); + return { r, cw, fetchCalls, beaconCalls }; + } + + it('replies to ready (from the iframe) with the config + nonce', () => { + const { r, cw } = setup(); + r.onMessage({ source: cw, data: { type: 'scorm', action: 'ready' } }); + expect(cw.postMessage).toHaveBeenCalledTimes(1); + expect(cw.postMessage.mock.calls[0][0]).toMatchObject({ type: 'scorm', action: 'config', nonce: 'N', teachermodevisible: 0 }); + }); + + it('forwards a valid track message to track.php with the trusted identity', () => { + const { r, cw, fetchCalls } = setup(); + r.onMessage({ + source: cw, + data: { type: 'scorm', action: 'track', exelearningBridge: 'N', cmi: { 'cmi.core.score.raw': '80' }, itemscores: { 'ide-a': { scorepct: 80 } } }, + }); + expect(fetchCalls).toHaveLength(1); + expect(fetchCalls[0].url).toBe('/track.php?id=42'); + const body = JSON.parse(fetchCalls[0].opts.body); + expect(body).toEqual({ id: 42, session: 'tok', cmi: { 'cmi.core.score.raw': '80' }, itemscores: { 'ide-a': { scorepct: 80 } } }); + }); + + it('ignores a track message from a window other than the iframe', () => { + const { r, fetchCalls } = setup(); + r.onMessage({ source: { foreign: true }, data: { type: 'scorm', action: 'track', exelearningBridge: 'N', cmi: {} } }); + expect(fetchCalls).toHaveLength(0); + }); + + it('ignores a track message with the wrong nonce or a bad shape', () => { + const { r, cw, fetchCalls } = setup(); + r.onMessage({ source: cw, data: { type: 'scorm', action: 'track', exelearningBridge: 'WRONG', cmi: {} } }); + r.onMessage({ source: cw, data: { type: 'scorm', action: 'track', exelearningBridge: 'N' } }); // no cmi + r.onMessage({ source: cw, data: { type: 'scorm', action: 'evil', exelearningBridge: 'N', cmi: {} } }); + expect(fetchCalls).toHaveLength(0); + }); + + it('flushBeacon sends the last forwarded payload on unload', () => { + const { r, cw, beaconCalls } = setup(); + r.onMessage({ source: cw, data: { type: 'scorm', action: 'track', exelearningBridge: 'N', cmi: { 'cmi.core.score.raw': '90' }, itemscores: {} } }); + r.flushBeacon(); + expect(beaconCalls).toHaveLength(1); + expect(beaconCalls[0].url).toBe('/track.php?id=42'); + }); +}); diff --git a/tests/player_iframe_test.php b/tests/player_iframe_test.php new file mode 100644 index 0000000..e27c203 --- /dev/null +++ b/tests/player_iframe_test.php @@ -0,0 +1,99 @@ +. + +namespace mod_exelearning; + +use advanced_testcase; +use mod_exelearning\local\ui\player_iframe; + +/** + * Tests for the package iframe security mode + sandbox policy (DEC-0059). + * + * @package mod_exelearning + * @category test + * @copyright 2026 ATE (Área de Tecnología Educativa) + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \mod_exelearning\local\ui\player_iframe + */ +final class player_iframe_test extends advanced_testcase { + /** + * The default (no config set) must be the secure, isolated mode. + */ + public function test_default_mode_is_secure(): void { + $this->resetAfterTest(); + $this->assertSame(player_iframe::MODE_SECURE, player_iframe::resolve_mode()); + $this->assertTrue(player_iframe::is_secure()); + } + + /** + * An unset/invalid config must fail safe to secure, never weakening isolation. + */ + public function test_invalid_mode_falls_back_to_secure(): void { + $this->resetAfterTest(); + set_config('iframemode', 'not-a-real-mode', 'mod_exelearning'); + $this->assertSame(player_iframe::MODE_SECURE, player_iframe::resolve_mode()); + $this->assertTrue(player_iframe::is_secure()); + } + + /** + * The legacy fallback is honoured when explicitly configured. + */ + public function test_legacy_mode_is_respected(): void { + $this->resetAfterTest(); + set_config('iframemode', player_iframe::MODE_LEGACY, 'mod_exelearning'); + $this->assertSame(player_iframe::MODE_LEGACY, player_iframe::resolve_mode()); + $this->assertFalse(player_iframe::is_secure()); + } + + /** + * Secure mode runs in an opaque origin: it MUST drop allow-same-origin and + * allow-popups-to-escape-sandbox, and MUST keep the scripts/popups/forms the + * iDevices need. It must never grant top navigation or modals. + */ + public function test_secure_sandbox_tokens(): void { + $tokens = player_iframe::sandbox_tokens(player_iframe::MODE_SECURE); + $list = explode(' ', $tokens); + + $this->assertContains('allow-scripts', $list); + $this->assertContains('allow-popups', $list); + $this->assertContains('allow-forms', $list); + + $this->assertNotContains('allow-same-origin', $list); + $this->assertNotContains('allow-popups-to-escape-sandbox', $list); + $this->assertNotContains('allow-top-navigation', $list); + $this->assertNotContains('allow-top-navigation-by-user-activation', $list); + $this->assertNotContains('allow-modals', $list); + } + + /** + * Legacy mode keeps the historical same-origin tokens (incl. allow-same-origin + * and allow-popups-to-escape-sandbox) but still never grants top navigation or + * modals. + */ + public function test_legacy_sandbox_tokens(): void { + $tokens = player_iframe::sandbox_tokens(player_iframe::MODE_LEGACY); + $list = explode(' ', $tokens); + + $this->assertContains('allow-scripts', $list); + $this->assertContains('allow-same-origin', $list); + $this->assertContains('allow-popups', $list); + $this->assertContains('allow-forms', $list); + $this->assertContains('allow-popups-to-escape-sandbox', $list); + + $this->assertNotContains('allow-top-navigation', $list); + $this->assertNotContains('allow-modals', $list); + } +} From 377fb1f5407a725a66aa5392633f9289366c4b32 Mon Sep 17 00:00:00 2001 From: erseco Date: Sat, 13 Jun 2026 09:12:23 +0100 Subject: [PATCH 07/35] docs(research): DEC-0059 secure SCORM postMessage bridge (advances DEC-0019) --- README.md | 20 +- ...9-bridge-scorm-postmessage-origen-opaco.md | 202 ++++++++++++++++++ research/docs/indices/adrs.yaml | 1 + research/docs/indices/diario.yaml | 1 + research/status.yaml | 51 ++++- ...2026-06-13-secure-iframe-scorm-bridge.yaml | 45 ++++ 6 files changed, 315 insertions(+), 5 deletions(-) create mode 100644 research/decisiones/adr/DEC-0059-bridge-scorm-postmessage-origen-opaco.md create mode 100644 research/tareas/diario/2026-06-13-secure-iframe-scorm-bridge.yaml diff --git a/README.md b/README.md index 7582765..914b42a 100644 --- a/README.md +++ b/README.md @@ -155,6 +155,13 @@ editor remains): editor's built-in styles, and optionally block users from importing styles bundled inside an `.elpx`. A dedicated _Styles_ admin page lists and manages them. +* **Package iframe security mode** (`iframemode`, default **Secure**): in _Secure_ + mode the eXeLearning package runs in a sandboxed, **opaque-origin** iframe so its + JavaScript cannot read or modify the surrounding Moodle page, its cookies or the + session; SCORM scoring is relayed to Moodle over a validated `postMessage` bridge. + _Legacy_ keeps the previous same-origin behaviour as a compatibility fallback (use + it only if a specific package misbehaves under an opaque origin). See + [DEC-0059](./research/decisiones/adr/DEC-0059-bridge-scorm-postmessage-origen-opaco.md). ## Embedded editor management @@ -191,10 +198,15 @@ attempts, and delete attempts from the teacher report (the grade is recalculated). Completion can require a passing grade (SCORM-style, see [DEC-0010](./research/decisiones/adr/DEC-0010-finalizacion-estilo-scorm.md)). -Grading runtime uses a SCORM 1.2 bridge: a small `window.API` shim installed by -`view.php` accepts `LMSSetValue` calls from the iDevice's bundled pipwerks -wrapper and forwards them to `track.php`, which calls Moodle's `grade_update()`. -xAPI support via `core_xapi` is on the roadmap. +Grading runtime uses a SCORM 1.2 bridge whose isolation depends on the **package +iframe security mode** +([DEC-0059](./research/decisiones/adr/DEC-0059-bridge-scorm-postmessage-origen-opaco.md)). +In the default **Secure** mode the package runs in an opaque-origin sandboxed iframe: +a `window.API` shim lives _inside_ the iframe and posts buffered scores to the Moodle +page over a validated `postMessage` channel; the page (which holds the `sesskey`) +forwards them to `track.php`, which calls Moodle's `grade_update()`. In **Legacy** mode +the shim is installed by `view.php` in the same-origin parent and the iDevice's bundled +pipwerks wrapper reaches it directly. xAPI support via `core_xapi` is on the roadmap. ## Web services (Mobile API) diff --git a/research/decisiones/adr/DEC-0059-bridge-scorm-postmessage-origen-opaco.md b/research/decisiones/adr/DEC-0059-bridge-scorm-postmessage-origen-opaco.md new file mode 100644 index 0000000..4feb535 --- /dev/null +++ b/research/decisiones/adr/DEC-0059-bridge-scorm-postmessage-origen-opaco.md @@ -0,0 +1,202 @@ +--- +id: DEC-0059 +titulo: "Bridge SCORM por postMessage + iframe de origen opaco (modo seguro configurable): implementación de RIE-001 Tier 2" +estado: Aceptada +fecha: 2026-06-13 +agentes: + - erseco + - claude-code +fuentes: + - REPO-002 + - REPO-004 + - REPO-005 +experimentos: [] +relacionados: + - RIE-001 + - AN-008 + - DEC-0019 + - DEC-0017 + - DEC-0042 + - TAREA-017 +herramienta_ia: + interfaz: claude-code + modelo: claude-opus-4-8 +--- + +## Contexto + +`DEC-0019` cerró la investigación de RIE-001 (XSS cross-component desde un `.elpx` +malicioso) y dejó dos tiers documentados pero **sin implementar por decisión del +usuario**. Su roadmap fijaba explícitamente que el aislamiento real (Tier 2) iría en +**"su propio ADR"**: `M6 — reescribir el bridge a postMessage → M7 — origen opaco`. + +Esta ADR es ese ADR. El usuario priorizó implementar el **bridge seguro** completo +(no sólo el endurecimiento Tier 1), configurable y con valor por defecto seguro, tras +confirmar que es la única alternativa que cumple los objetivos de aislamiento (el +contenido NO puede leer ni modificar el DOM/cookies/sesión/JS del marco padre de +Moodle). **No supersede a DEC-0019**: la implementa/avanza. + +El problema de fondo (ya documentado en DEC-0019) es que el SCORM era 100% +same-origin, con **tres dependencias duras** que hacían que quitar `allow-same-origin` +matase el tracking en silencio: + +1. el **padre** inyectaba `window.API` y el hijo (pipwerks) lo buscaba recorriendo + `window.parent`; +2. el **padre** leía `iframe.contentDocument` para mapear el `objectid` de cada + iDevice (DEC-0017); +3. el **teacher-mode hider** inyectaba ` + +
+ +
+ + + + diff --git a/tests/js/exe_embed.test.js b/tests/js/exe_embed.test.js index b9aeff5..ffd7955 100644 --- a/tests/js/exe_embed.test.js +++ b/tests/js/exe_embed.test.js @@ -139,3 +139,36 @@ describe('exe_embed_shim promotion decisions', () => { expect(reported).toMatch(/files\/local-sample\.pdf$/); }); }); + +describe('exe_embed_relay makePlayer() attributes', () => { + it('builds a video player with autoplay/fullscreen grants + strict-origin referrer', () => { + const frame = relay.makePlayer({ url: 'https://www.youtube-nocookie.com/embed/abc123', kind: 'video' }); + expect(frame.tagName).toBe('IFRAME'); + expect(frame.getAttribute('allow')).toContain('autoplay'); + expect(frame.getAttribute('allow')).toContain('fullscreen'); + expect(frame.hasAttribute('allowfullscreen')).toBe(true); + expect(frame.getAttribute('referrerpolicy')).toBe('strict-origin-when-cross-origin'); + expect(frame.src).toContain('youtube-nocookie.com'); + }); + + it('builds a PDF player with no-referrer and no autoplay grant', () => { + const frame = relay.makePlayer({ url: 'https://files.test/manual.pdf', kind: 'pdf' }); + expect(frame.getAttribute('referrerpolicy')).toBe('no-referrer'); + expect(frame.getAttribute('allow') || '').not.toContain('autoplay'); + }); +}); + +describe('exe_embed_shim collect() geometry report', () => { + it('reports id + (absolute) url + numeric geometry for each placeholder', () => { + const root = document.createElement('div'); + root.innerHTML = + ''; + shim.promote(root, HOSTS, { n: 0 }); + + const embeds = shim.collect(root); + expect(embeds.length).toBe(1); + expect(embeds[0].id).toMatch(/^exe-embed-/); + expect(embeds[0].url).toContain('youtube-nocookie.com'); + ['x', 'y', 'w', 'h'].forEach((k) => expect(typeof embeds[0][k]).toBe('number')); + }); +}); From fed21b70e39935f437c8b0a00fec609bed8bbaa7 Mon Sep 17 00:00:00 2001 From: erseco Date: Sun, 14 Jun 2026 06:54:20 +0100 Subject: [PATCH 22/35] Fix CodeQL alert in the e2e spec + lift embed JS coverage past the patch gate - CodeQL (high): the e2e spec asserted a non-whitelisted host was absent with src.includes('example.com') ("incomplete URL substring sanitization"). Rewrite the assertions to parse exact hostnames (new URL().hostname) and an anchored regex for the canonical YouTube URL, so no URL is checked by substring. - Codecov patch: cover the relay's createRelay()/onMessage -> overlay player path and the instance validate() in Vitest, and mark the browser-only bootstrap of both the shim (init/report/observer) and the relay (init/pingAll/scheduleReflow) as v8-ignore (they require a framed, opaque-origin window and are exercised by the Firefox e2e, not happy-dom). exe_embed_relay.js 95% / exe_embed_shim.js 84% line coverage; 77 unit tests. --- js/exe_embed_relay.js | 8 ++++++ js/exe_embed_shim.js | 4 +++ tests/e2e/embed.spec.cjs | 11 ++++++--- tests/js/exe_embed.test.js | 50 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 4 deletions(-) diff --git a/js/exe_embed_relay.js b/js/exe_embed_relay.js index d073e0e..b7b737e 100644 --- a/js/exe_embed_relay.js +++ b/js/exe_embed_relay.js @@ -254,6 +254,10 @@ sync(overlayFor(iframe), data.embeds, iframe.src); } + // Browser-only glue below (window listeners, reflow on scroll/resize, pinging + // the content iframes). Exercised by the Playwright/Firefox e2e + // (tests/e2e/embed.spec.cjs), not the happy-dom unit tests. + /* v8 ignore start */ function pingAll() { var frames = document.getElementsByTagName('iframe'); for (var i = 0; i < frames.length; i++) { @@ -278,6 +282,7 @@ } }); } + /* v8 ignore stop */ return { onMessage: onMessage, @@ -285,6 +290,7 @@ validate: function (raw, contentSrc) { return validate(raw, contentSrc, whitelist); }, + /* v8 ignore start */ init: function () { window.addEventListener('message', onMessage); window.addEventListener('resize', scheduleReflow); @@ -294,6 +300,7 @@ window.setTimeout(pingAll, 500); return this; } + /* v8 ignore stop */ }; } @@ -303,6 +310,7 @@ * @param {Object} config {whitelist: string[]} * @returns {Object} */ + /* v8 ignore next 3 */ function init(config) { return createRelay(config).init(); } diff --git a/js/exe_embed_shim.js b/js/exe_embed_shim.js index a845bcc..c586ac7 100644 --- a/js/exe_embed_shim.js +++ b/js/exe_embed_shim.js @@ -183,7 +183,10 @@ /** * Bootstrap inside the package iframe (no-op outside the secure opaque origin). + * Browser-only glue (requires a framed, opaque-origin window); exercised by the + * Playwright/Firefox e2e (tests/e2e/embed.spec.cjs), not the happy-dom unit tests. */ + /* v8 ignore start */ function init() { if (window.parent === window || !isOpaqueOrigin()) { return; @@ -230,6 +233,7 @@ } }); } + /* v8 ignore stop */ var exp = { isOpaqueOrigin: isOpaqueOrigin, diff --git a/tests/e2e/embed.spec.cjs b/tests/e2e/embed.spec.cjs index 72c3ede..47e55e5 100644 --- a/tests/e2e/embed.spec.cjs +++ b/tests/e2e/embed.spec.cjs @@ -29,11 +29,14 @@ test('promotes whitelisted video + relative local PDF to inline parent players ( await expect.poll(() => players.count(), { timeout: 15000 }).toBe(2); const srcs = await players.evaluateAll((els) => els.map((e) => e.src)); + const hosts = srcs.map((s) => { + try { return new URL(s).hostname.toLowerCase(); } catch (e) { return ''; } + }); - // The video is rebuilt to the canonical nocookie URL. - expect(srcs.some((s) => s.includes('youtube-nocookie.com/embed/aqz-KE-bpKQ'))).toBe(true); + // The video is rebuilt to the canonical nocookie URL (anchored, not a substring match). + expect(srcs.some((s) => /^https:\/\/www\.youtube-nocookie\.com\/embed\/aqz-KE-bpKQ\b/.test(s))).toBe(true); // The relative local PDF is reported absolute and rendered (the relative-URL fix). expect(srcs.some((s) => /\/local\.pdf$/.test(s))).toBe(true); - // The non-whitelisted iframe is never promoted. - expect(srcs.some((s) => s.includes('example.com'))).toBe(false); + // The non-whitelisted host is never promoted (exact hostname check, not a substring). + expect(hosts).not.toContain('example.com'); }); diff --git a/tests/js/exe_embed.test.js b/tests/js/exe_embed.test.js index ffd7955..d945333 100644 --- a/tests/js/exe_embed.test.js +++ b/tests/js/exe_embed.test.js @@ -158,6 +158,56 @@ describe('exe_embed_relay makePlayer() attributes', () => { }); }); +describe('exe_embed_relay createRelay() overlays players from messages', () => { + let iframe; + beforeEach(() => { + document.body.innerHTML = ''; + iframe = document.createElement('iframe'); + document.body.appendChild(iframe); + }); + + it('creates an inline overlay player for a valid embed and removes it when no longer reported', () => { + const r = relay.createRelay({ whitelist: HOSTS }); + // The instance validate() binds the configured whitelist. + expect(r.validate('https://www.youtube.com/embed/abc123', 'http://x/content/1/index.html')) + .toMatchObject({ kind: 'video' }); + r.onMessage({ + source: iframe.contentWindow, + data: { + type: 'exe-embed', + action: 'sync', + embeds: [{ id: 'e1', url: 'https://www.youtube.com/embed/abc123', x: 0, y: 0, w: 480, h: 270 }], + }, + }); + const players = document.querySelectorAll('.exe-embed-overlay iframe'); + expect(players.length).toBe(1); + expect(players[0].src).toMatch(/youtube-nocookie\.com\/embed\/abc123$/); + + // Stale removal: a later sync without the embed removes its player. + r.onMessage({ source: iframe.contentWindow, data: { type: 'exe-embed', action: 'sync', embeds: [] } }); + expect(document.querySelectorAll('.exe-embed-overlay iframe').length).toBe(0); + }); + + it('ignores a message whose source is not a known content iframe', () => { + const r = relay.createRelay({ whitelist: HOSTS }); + r.onMessage({ + source: {}, + data: { + type: 'exe-embed', + action: 'sync', + embeds: [{ id: 'x', url: 'https://www.youtube.com/embed/abc123', x: 0, y: 0, w: 1, h: 1 }], + }, + }); + expect(document.querySelectorAll('.exe-embed-overlay iframe').length).toBe(0); + }); + + it('ignores non-embed messages', () => { + const r = relay.createRelay({ whitelist: HOSTS }); + r.onMessage({ source: iframe.contentWindow, data: { type: 'scorm', action: 'track', cmi: {} } }); + expect(document.querySelectorAll('.exe-embed-overlay iframe').length).toBe(0); + }); +}); + describe('exe_embed_shim collect() geometry report', () => { it('reports id + (absolute) url + numeric geometry for each placeholder', () => { const root = document.createElement('div'); From 19a912a90fa7b44fa2af47697d14c8c62bed4044 Mon Sep 17 00:00:00 2001 From: erseco Date: Sun, 14 Jun 2026 11:15:18 +0100 Subject: [PATCH 23/35] Fix secure-mode SCORM saving: pipwerks get() must check the local window first The vendored pipwerks API.get() started its lookup at win.parent and skipped the current window. In secure (opaque-origin) mode the SCORM API is provided locally as window.API by the in-iframe bridge shim (DEC-0059) and the Moodle parent is a cross-origin/opaque frame, so reaching into parent.API threw SecurityError, init() never activated the connection, and every LMSSetValue/LMSCommit became a silent no-op -- no attempt rows were ever written. Legacy (same-origin) mode masked it because the parent there hosts the API. Restore the standard pipwerks order: try find(window) first, then fall back to the parent and opener, with every cross-origin hop wrapped so an opaque ancestor can never abort the lookup. Legacy keeps working: with no local API, find(window) walks up to the same-origin parent exactly as before. Add tests/js/scorm_api_wrapper.test.js (loads the vendored wrapper with a controllable window) covering current-window-first, opaque-parent safety, the null fallback, the legacy walk-up, and the full init()/set() save path. Document the root cause and the DEC-0059 verification gap in DEC-0062. --- assets/scorm/SCORM_API_wrapper.js | 31 +++- ...fix-pipwerks-get-api-local-origen-opaco.md | 144 +++++++++++++++ tests/js/scorm_api_wrapper.test.js | 166 ++++++++++++++++++ 3 files changed, 337 insertions(+), 4 deletions(-) create mode 100644 research/decisiones/adr/DEC-0062-fix-pipwerks-get-api-local-origen-opaco.md create mode 100644 tests/js/scorm_api_wrapper.test.js diff --git a/assets/scorm/SCORM_API_wrapper.js b/assets/scorm/SCORM_API_wrapper.js index 6ea6c37..34134b0 100644 --- a/assets/scorm/SCORM_API_wrapper.js +++ b/assets/scorm/SCORM_API_wrapper.js @@ -133,12 +133,35 @@ pipwerks.SCORM.API.get = function () { find = pipwerks.SCORM.API.find, trace = pipwerks.UTILS.trace; - if (win.parent && win.parent != win) { - API = find(win.parent); + // Check the CURRENT window's frame hierarchy first (standard pipwerks order). In + // the secure (opaque-origin) package mode the SCORM API is provided locally by the + // in-iframe bridge shim (js/scorm_bridge_shim.js, DEC-0059) as window.API, and the + // Moodle parent is a cross-origin/opaque frame that throws SecurityError on access. + // Starting at win.parent (as the prior build did) made init() throw there and the + // connection never went active, so no score was ever saved in secure mode. find(win) + // returns the local API when present and otherwise walks up same-origin ancestors, + // which keeps the legacy same-origin mode (API hosted by the Moodle parent) working. + // Every cross-origin hop is wrapped so an opaque ancestor can never abort lookup. + try { + API = find(win); + } catch (e) { + trace("API.get: find(window) threw: " + e); + } + + if (!API && win.parent && win.parent != win) { + try { + API = find(win.parent); + } catch (e) { + trace("API.get: find(parent) blocked (cross-origin): " + e); + } } - if (!API && win.top.opener) { - API = find(win.top.opener); + try { + if (!API && win.top && win.top.opener) { + API = find(win.top.opener); + } + } catch (e) { + trace("API.get: find(opener) blocked: " + e); } if (API) { diff --git a/research/decisiones/adr/DEC-0062-fix-pipwerks-get-api-local-origen-opaco.md b/research/decisiones/adr/DEC-0062-fix-pipwerks-get-api-local-origen-opaco.md new file mode 100644 index 0000000..bea9743 --- /dev/null +++ b/research/decisiones/adr/DEC-0062-fix-pipwerks-get-api-local-origen-opaco.md @@ -0,0 +1,144 @@ +--- +id: DEC-0062 +titulo: "Fix: el bridge SCORM seguro nunca guardaba nota porque el get() de pipwerks no miraba la ventana local" +estado: Aceptada +fecha: 2026-06-14 +agentes: + - erseco + - claude-code +fuentes: + - REPO-002 +experimentos: [] +relacionados: + - DEC-0059 + - DEC-0060 + - DEC-0017 + - DEC-0042 + - RIE-001 +herramienta_ia: + interfaz: claude-code + modelo: claude-opus-4-8 +--- + +## Contexto + +`DEC-0059` implementó el bridge SCORM por `postMessage` con el iframe en **origen +opaco** (modo `secure`, por defecto). Su pieza clave ("el linchpin") era inyectar un +`window.API` (shim) DENTRO del iframe para que el pipwerks empaquetado lo descubriese +**sin tocar el padre** cross-origin. + +Al verificar en vivo (`localhost:80`, modo `secure`) que la puntuación SCORM se +guardaba, erseco reportó que **NO se guardaba**: contestar una pregunta y pulsar +"Guardar la puntuación" no creaba ningún intento en `exelearning_attempt` ni llegaba +ninguna petición a `track.php`. En modo `legacy` sí se guardaba. La "verificación" +previa de DEC-0059 había usado un POST directo a `track.php` que **saltaba el bridge**, +enmascarando el fallo. + +## Problema + +¿Por qué en modo `secure` el SCO nunca persiste la nota, si el shim sí inyecta +`window.API` en el iframe y el handshake del bridge (`ready`→`config`) funciona? + +## Hechos verificados (con cita) + +Diagnóstico en vivo instrumentando el shim del iframe opaco (no observable desde fuera) +y emitiendo el estado de pipwerks por `postMessage` al padre. Resultados decisivos sobre +una actividad evaluable real en modo `secure`: + +1. **El shim funciona:** `window.API === shim` (`apiIsShim:true`), el handshake llega + (sin "secure blocked"), `pipwerks` cargó (`typeof === 'object'`). +2. **Pero la conexión nunca se activa:** `pipwerks.SCORM.connection.isActive === false`, + `pipwerks.SCORM.version === null`, `pipwerks.SCORM.API.isFound === false`, + `pipwerks.SCORM.API.handle == null`, y `LMSInitialize` **nunca se llamó** (contador + 0). Por tanto `LMSSetValue`/`LMSCommit` del guardado son no-ops silenciosos → 0 filas. +3. **Causa raíz — `get()` lanza `SecurityError` y nunca mira la ventana local.** + Forzando `pipwerks.SCORM.API.get()` desde el shim: + `"SecurityError: Failed to read a named property 'API' from 'Window': Blocked a frame + with origin \"null\" from accessing a cross-origin frame."` + El `get()` vendado (`assets/scorm/SCORM_API_wrapper.js:130-150`, **modificado** + respecto al pipwerks estándar) arrancaba directamente en el **padre** y **omitía la + ventana actual**: + ```js + if (win.parent && win.parent != win) { API = find(win.parent); } // ← sin find(win) + if (!API && win.top.opener) { API = find(win.top.opener); } + ``` + - En `legacy` el padre es same-origin (Moodle, con `window.API`) → funciona. + - En `secure` el padre es opaco/cross-origin → `find(win.parent)` accede a + `parent.API` y lanza `SecurityError` → `init()` revienta (lo traga su `catch`) → + `isActive` se queda en `false` → nada se guarda. +4. **El "linchpin" de DEC-0059 verificó la función equivocada.** DEC-0059 §"Hechos + verificados" (1) cita `find(win)` (`SCORM_API_wrapper.js:71-106`), que sí parte de la + ventana actual. Pero el punto de entrada real es `getHandle()`→`get()`, y **este + `get()` no llamaba a `find(window)`**. La premisa "pipwerks descubre el `window.API` + local antes de subir" era cierta para `find()` pero **falsa para `get()`**, que es + quien decide. El origen opaco del modo `secure` nunca fue alcanzable. +5. **No hay doble pipwerks.** El export de eXeLearning sólo **usa** `pipwerks.SCORM.get/ + set` (`libs/common.js:945,1175`); el único `pipwerks` es el vendado por mod + (`var pipwerks = {}`), inyectado por `scorm_injector`. Descartado como causa. + +## Causa raíz + +El `get()` vendado de pipwerks fue alterado en algún momento para mirar **sólo el +padre** y saltarse la ventana actual. Eso es exactamente incompatible con la +arquitectura de DEC-0059, que provee el API **localmente** en un iframe cuyo padre es +**inalcanzable** por opaco. + +## Decisión + +Restaurar en `assets/scorm/SCORM_API_wrapper.js` el orden estándar de pipwerks en +`get()`: **mirar primero la ventana actual** (`find(win)`) y sólo después subir al padre +/ opener, **envolviendo cada salto cross-origin en `try/catch`** para que un ancestro +opaco no pueda abortar la búsqueda nunca más. + +```js +try { API = find(win); } catch (e) { trace(...); } // ventana local primero +if (!API && win.parent && win.parent != win) { + try { API = find(win.parent); } catch (e) { trace(...); } // fallback same-origin (legacy) +} +try { if (!API && win.top && win.top.opener) { API = find(win.top.opener); } } catch (e) { trace(...); } +``` + +- **`secure`:** `find(win)` encuentra el `window.API` del shim de inmediato (el bucle de + `find` no sube porque `win.API` existe) → `init()` activa la conexión → el guardado + fluye shim → `postMessage('track')` → relevo → `track.php` → BD. +- **`legacy`:** sin API local, `find(win)` **sube** al padre same-origin y devuelve su + `window.API`, idéntico al comportamiento previo (equivalencia probada en test). + +Cambio mínimo y acotado a `get()`; no se toca `find()`, ni el shim, ni el relevo, ni el +inyector, ni BD/upgrade. + +## Consecuencias + +- **Positiva:** el modo `secure` por defecto (DEC-0059/DEC-0060) **por fin guarda la + nota SCORM** de extremo a extremo; el "linchpin" que DEC-0059 daba por hecho ahora se + cumple de verdad. El aislamiento de RIE-001 se mantiene intacto (el `sesskey` sigue en + el padre; el contenido sigue sin alcanzarlo). +- **Endurecimiento:** `get()` ya no puede lanzar por un ancestro opaco; degrada a `null` + con traza, como manda el contrato de pipwerks. +- **Sin regresión en `legacy`:** la búsqueda sigue resolviendo al `window.API` del padre + same-origin (sube por `find`), verificado por test unitario. + +## Validación + +- **Vivo (`localhost:80`, modo `secure`):** tras contestar + "Guardar la puntuación", se + escriben las filas reales en `exelearning_attempt` (`rawscore=100.00`, + `status=completed`/`passed`) y el bridge emite `track` (`hasCmi:true`). Antes del fix: + 0 filas, 0 `track`. +- **Vitest** `tests/js/scorm_api_wrapper.test.js` (nuevo, 6 casos, verde): `get()` + devuelve el API **local** y no el del padre (regresión del bug); `get()` no lanza con + padre opaco y encuentra el local; `get()` devuelve `null` sin lanzar cuando no hay API + y el padre es opaco; `get()` sigue subiendo a un padre same-origin sin API local + (legacy); `init()` activa la conexión vía API local con padre opaco; `set()` llega al + API local una vez activa (la nota se guarda). Suite JS completa **87/87**. +- **phpcs/PHPUnit/Behat**: el fix es JS vendado (sin PHP); se confirma en CI. + +## Seguimiento + +- **Test de regresión vendado:** este ADR introduce el primer test Vitest sobre + `assets/scorm/*` (declarado "fuera de alcance" en `vitest.config`), justificado por ser + crítico para la nota. No cuenta para cobertura (`include: ['js/**']`); es guardia de + comportamiento. +- **Lección para DEC-0059:** verificar el **punto de entrada real** (`getHandle`/`get`), + no sólo la función auxiliar (`find`), antes de declarar un linchpin. +- Posible follow-up: un Behat `@javascript` de grading que corra en modo `secure` real + (no sólo el paso server-side) habría cazado esto antes; queda recomendado. diff --git a/tests/js/scorm_api_wrapper.test.js b/tests/js/scorm_api_wrapper.test.js new file mode 100644 index 0000000..11bc552 --- /dev/null +++ b/tests/js/scorm_api_wrapper.test.js @@ -0,0 +1,166 @@ +// This file is part of Moodle - http://moodle.org/ +// +// Moodle is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Moodle is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Moodle. If not, see . + +// Regression coverage for the vendored pipwerks wrapper (assets/scorm/SCORM_API_wrapper.js). +// +// The wrapper is normally "out of scope" for these JS suites, but its API lookup is +// grade-critical: it is what eXeLearning's content calls to record SCORM scores, and a +// defect there silently lost EVERY score in the secure (opaque-origin) iframe mode +// (DEC-0059/DEC-0061). The vendored get() had been altered to look only in window.parent +// and skip the current window. In secure mode the SCORM API is provided LOCALLY by the +// in-iframe bridge shim (js/scorm_bridge_shim.js) as window.API, and the Moodle parent is +// a cross-origin/opaque frame whose property access throws SecurityError. So the altered +// get() reached straight into the opaque parent, threw, init() never activated the +// connection, and LMSSetValue/LMSCommit became no-ops -> no attempt rows were written. +// +// These tests lock in the corrected behaviour: check the current window first, fall back +// to a same-origin ancestor (legacy mode), and never let an opaque ancestor abort lookup. +// +// The wrapper is a classic browser script (top-level `var pipwerks = {}`, no module +// exports), so it is loaded via new Function() with a fully controllable `window`, letting +// each test model a specific frame topology (local API, opaque parent, same-origin parent). + +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const here = path.dirname(fileURLToPath(import.meta.url)); +const wrapperSrc = fs.readFileSync( + path.join(here, '../../assets/scorm/SCORM_API_wrapper.js'), + 'utf8' +); + +/** + * Load a fresh pipwerks instance bound to the given fake window. find()/get() read the + * `window` free variable, which we supply as the factory argument, so the wrapper sees + * exactly the frame topology the test sets up. A fresh instance per call avoids shared + * state (pipwerks.SCORM.version, connection.isActive) leaking between tests. + * + * @param {Object} win The fake window (must carry a console for trace()). + * @returns {Object} The wrapper's pipwerks namespace. + */ +function loadPipwerks(win) { + win.console = win.console || globalThis.console; + const factory = new Function('window', 'document', wrapperSrc + '\n;return pipwerks;'); + return factory(win, win.document || {}); +} + +/** + * A minimal SCORM 1.2 API surface. Records LMSInitialize/LMSSetValue/LMSCommit calls so a + * test can assert the save path was actually driven. LMSGetLastError returns "0" so + * pipwerks treats the session as healthy. + * + * @param {string} tag An identity marker so tests can assert WHICH API was returned. + * @returns {Object} + */ +function makeApi(tag) { + const calls = []; + return { + tag, + calls, + LMSInitialize: (v) => { calls.push(['LMSInitialize', v]); return 'true'; }, + LMSFinish: (v) => { calls.push(['LMSFinish', v]); return 'true'; }, + LMSGetValue: (k) => { calls.push(['LMSGetValue', k]); return ''; }, + LMSSetValue: (k, v) => { calls.push(['LMSSetValue', k, v]); return 'true'; }, + LMSCommit: (v) => { calls.push(['LMSCommit', v]); return 'true'; }, + LMSGetLastError: () => '0', + LMSGetErrorString: () => '', + LMSGetDiagnostic: () => '', + }; +} + +/** A frame whose .API / .API_1484_11 access throws, mimicking an opaque cross-origin parent. */ +function makeOpaqueFrame() { + const frame = {}; + const blow = () => { throw new Error('SecurityError: Blocked a frame with origin "null" from accessing a cross-origin frame.'); }; + Object.defineProperty(frame, 'API', { get: blow }); + Object.defineProperty(frame, 'API_1484_11', { get: blow }); + return frame; +} + +describe('vendored pipwerks API.get (secure-mode regression)', () => { + it('returns the LOCAL window.API, not the parent\'s (current window checked first)', () => { + // Secure mode: the bridge shim put window.API in THIS frame; the parent also has a + // (different) API. The old build returned the parent's; we must return the local one. + const localApi = makeApi('local'); + const parent = { API: makeApi('parent') }; + parent.parent = parent; // stops the upward walk at the parent + const win = { API: localApi, parent, document: {} }; + + const pipwerks = loadPipwerks(win); + + expect(pipwerks.SCORM.API.get()).toBe(localApi); + }); + + it('does not throw when the parent frame is opaque/cross-origin, and finds the local API', () => { + // The exact failure mode: reaching into the opaque parent threw SecurityError. + const localApi = makeApi('local'); + const win = { API: localApi, parent: makeOpaqueFrame(), document: {} }; + + const pipwerks = loadPipwerks(win); + + let got; + expect(() => { got = pipwerks.SCORM.API.get(); }).not.toThrow(); + expect(got).toBe(localApi); + }); + + it('returns null (never throws) when no API is reachable and the parent is opaque', () => { + const win = { parent: makeOpaqueFrame(), document: {} }; // no local API + + const pipwerks = loadPipwerks(win); + + let got = 'unset'; + expect(() => { got = pipwerks.SCORM.API.get(); }).not.toThrow(); + expect(got == null).toBe(true); + }); + + it('still walks up to a same-origin parent when there is no local API (legacy mode)', () => { + // Legacy same-origin mode: no local API, the Moodle parent hosts window.API. + const parentApi = makeApi('parent'); + const parent = { API: parentApi }; + parent.parent = parent; // stops the walk + const win = { parent, document: {} }; + + const pipwerks = loadPipwerks(win); + + expect(pipwerks.SCORM.API.get()).toBe(parentApi); + }); +}); + +describe('vendored pipwerks init (secure bridge end to end)', () => { + it('activates the connection through a LOCAL API even with an opaque parent', () => { + const localApi = makeApi('local'); + const win = { API: localApi, parent: makeOpaqueFrame(), document: {} }; + + const pipwerks = loadPipwerks(win); + const ok = pipwerks.SCORM.init(); // init === connection.initialize + + expect(ok).toBe(true); + expect(pipwerks.SCORM.connection.isActive).toBe(true); + expect(localApi.calls.some((c) => c[0] === 'LMSInitialize')).toBe(true); + }); + + it('set() reaches the local API once the connection is active (the score actually saves)', () => { + const localApi = makeApi('local'); + const win = { API: localApi, parent: makeOpaqueFrame(), document: {} }; + + const pipwerks = loadPipwerks(win); + pipwerks.SCORM.init(); + const saved = pipwerks.SCORM.set('cmi.core.score.raw', '100'); + + expect(saved).toBe(true); + expect(localApi.calls).toContainEqual(['LMSSetValue', 'cmi.core.score.raw', '100']); + }); +}); From c8df0333775dae3f2475b1172a4a093b3b951695 Mon Sep 17 00:00:00 2001 From: erseco Date: Sun, 14 Jun 2026 11:15:29 +0100 Subject: [PATCH 24/35] Add Dailymotion + EducaMadrid embed providers and clamp the overlay geometry Extend the external-embed allowlist with Dailymotion (www/geo.dailymotion.com, /embed/video/{id}) and EducaMadrid/Mediateca de Madrid (mediateca.educa.madrid.org, /video/{id}/fs), each with a per-provider canonical-URL validator in the relay so only a reconstructed, known-good embed URL is ever rendered. Clamp the relayed player overlay to the placeholder's rect (defence in depth against geometry-driven clickjacking; the overlay already clips with overflow:hidden). Mark the shim/relay as the canonical source for the wp/omeka mirrors. Refresh the demo fixture and the Vitest + Firefox e2e coverage with the new hosts and their reject cases. --- classes/local/ui/player_iframe.php | 4 +++ js/exe_embed_relay.js | 23 ++++++++++++++++-- js/exe_embed_shim.js | 5 ++++ .../fixtures/elpx/external-embeds-demo.elpx | Bin 1958 -> 2024 bytes tests/e2e/embed.spec.cjs | 4 ++- tests/e2e/embed/content.html | 5 +++- tests/e2e/embed/parent.html | 4 ++- tests/js/exe_embed.test.js | 23 ++++++++++++++++++ 8 files changed, 63 insertions(+), 5 deletions(-) diff --git a/classes/local/ui/player_iframe.php b/classes/local/ui/player_iframe.php index 6b62990..5bf5542 100644 --- a/classes/local/ui/player_iframe.php +++ b/classes/local/ui/player_iframe.php @@ -66,6 +66,10 @@ final class player_iframe { 'youtube-nocookie.com', 'player.vimeo.com', 'vimeo.com', + 'www.dailymotion.com', + 'dailymotion.com', + 'geo.dailymotion.com', + 'mediateca.educa.madrid.org', ]; /** diff --git a/js/exe_embed_relay.js b/js/exe_embed_relay.js index b7b737e..b8a329d 100644 --- a/js/exe_embed_relay.js +++ b/js/exe_embed_relay.js @@ -29,6 +29,11 @@ * * Exposed two ways from a single body: window.exeEmbedRelay (browser bootstrap) and * module.exports (Vitest). See research ADR DEC-0059. + * + * CANONICAL SOURCE for the eXeLearning embedder family. wp-exelearning + * (assets/js/exe-embed-relay.js) and omeka-s-exelearning (asset/js/exe-embed-relay.js) + * mirror this logic (only the export wrapper differs: they are auto-running IIFEs). + * Keep the three in sync; tools/check-embed-sync.mjs fails if they drift. */ (function () { 'use strict'; @@ -120,6 +125,15 @@ match = url.pathname.match(/^\/video\/([0-9]+)$/); return match ? { url: 'https://player.vimeo.com/video/' + match[1], kind: 'video' } : null; } + if (host.indexOf('dailymotion') !== -1) { + match = url.pathname.match(/^\/embed\/video\/([A-Za-z0-9]{5,})$/); + return match ? { url: 'https://www.dailymotion.com/embed/video/' + match[1], kind: 'video' } : null; + } + if (host === 'mediateca.educa.madrid.org') { + // EducaMadrid / Mediateca de Madrid embed: /video/{id}/fs (the watch URL is /video/{id}). + match = url.pathname.match(/^\/video\/([A-Za-z0-9]{8,})(?:\/fs)?$/); + return match ? { url: 'https://mediateca.educa.madrid.org/video/' + match[1] + '/fs', kind: 'video' } : null; + } } if (/\.pdf$/i.test(url.pathname)) { @@ -229,10 +243,15 @@ entry.el.appendChild(player); entry.players[embed.id] = player; } + // Defence in depth against clickjacking: the overlay is clamped to the + // content iframe's box and clips with overflow:hidden, so a player can + // never cover host UI outside the iframe. Cap the player size to the + // overlay too (the content reports geometry, the parent owns rendering). + var rect = entry.iframe.getBoundingClientRect(); player.style.left = embed.x + 'px'; player.style.top = embed.y + 'px'; - player.style.width = embed.w + 'px'; - player.style.height = embed.h + 'px'; + player.style.width = Math.min(embed.w, rect.width) + 'px'; + player.style.height = Math.min(embed.h, rect.height) + 'px'; }); Object.keys(entry.players).forEach(function (id) { if (!seen[id]) { diff --git a/js/exe_embed_shim.js b/js/exe_embed_shim.js index c586ac7..e3bc6d8 100644 --- a/js/exe_embed_shim.js +++ b/js/exe_embed_shim.js @@ -31,6 +31,11 @@ * * Exposed two ways from a single body: window.exeEmbedShim (browser bootstrap) and * module.exports (Vitest). See research ADR DEC-0059. + * + * CANONICAL SOURCE for the eXeLearning embedder family. wp-exelearning + * (assets/js/exe-embed-shim.js) and omeka-s-exelearning (asset/js/exe-embed-shim.js) + * mirror this logic (only the export wrapper differs: they are auto-running IIFEs). + * Keep the three in sync; tools/check-embed-sync.mjs fails if they drift. */ (function () { 'use strict'; diff --git a/research/fixtures/elpx/external-embeds-demo.elpx b/research/fixtures/elpx/external-embeds-demo.elpx index f891e5f78ac84acdbf2bdea852888f38a022eae6..042c2fbd086390cd03876785a0f0f442c2edab59 100644 GIT binary patch delta 1054 zcmV+(1mXLp59kjKP)h>@6aWAK2mlpH&an+T0e=-q&Rk3SFKTWB006xR000UA003!j zWMz0RXmo9CwU%vf+cpr#-}_T=0ybp8B+7A|IE(BTX;Ktu({_V5psxx_JX@^!G?B98 ze)^G={IuHhhGDO09m%`j|L#mO`|jz(tNEvoJ5&jy(v5GQgujX*};0EEsm0hXYm zB!86wJB@yuzm6x-EVImVm_4@^DoHp3kpne|f>0Um<}w`i53*d*4JtS>#yM^whE}V} z`T&A3xt-Bu3C@O4M%fxMI*k~GH7g(%KpBbCG#y6U#r@Q0sVE=|t&I>Rmc-jFcB5e$ zg$eJ{IVF{4g{SgRP`UAX>nvAo9FU_kdVk96UX0)8=w^f9(|IjNvtQj33ux^^EA)e) z#FP){ld9)+4sO?Iwq45>6%l|MMr}Eb#XbZGU-~(FJoPV35 z0+0_#v%=OD#7xHh&wk&2k<;8EUzYnj6QE9sJ<_B(*+7-7{oqSLkyC(tfD-mH^qJOGoFR31n$N8F#^3}L8Broy8T$Xph_3z|;e0q(p zy6K-#VEpm21of41PLd_FWt}HXVNDmw7lRwfv8#}-z!;#3tw`ab$Q5j2%_)o1`^2r> zsTro}_j@@vX}XiMhl@_rzf*-fMgqJh#mXIS6ftR-Kd(wa32@z>7qVa+41ea{M{J1D z9R7xDbiL==g*}7Sb2H(9(A9}_>RHCb@;IHf=8DjyVJjAzBlLp*(*DaQ)aB4%mur#Z z5grlzir93Ne9=qn;B*`BzJ2W-a&>^QQ{{CyYJCd$&HUxvxNr3{*xlq2I8}`d=s5@J zPVWG&ffOaq*)p$-RbETE`F||bnz?*GvgM<*JqDYjX@`?E5&FhmJ4)J0aq}0{5b_)3 zt_FRI|6%bD(|%a|;3d&=n>fDrz3+zG6M1{DWZaa@0%ykh@LhLZiP~87{=*#QfOMsN za_IvymTU|%L{fJd0=@hXgSP8Rf7~4vaMzW?gRYdvFyHiMhW)LY;UMiLJ=mHC8TM}k zKl)dMAG^oOpHNE!1QY-O00;mTNzSvL1ZV*R6-myM2?m=36-myMrv^3-6-mxqOZqQr zZUX=Sy$F-*1}Y#GNzPnhk$6)#0RR9v0ssIO00000000000001h0n-GNtppg8O$Q7E Y6-myMTn8HhJ_eI{2PX#F1^@s60H}!Y4gdfE delta 965 zcmV;$13LWZ52g@6aWAK2mr>q&9MzS0e{B2&0H@6QS&SV0043a000UA003!j zWMz0RXmo9Ct(Du3+cpq}@BI|4fEHVz66JOF)Rld)$p!(ECTZao$W=gzvle5XN>Y}b zr|(df>~*v57HF?1@i6@Ue}-i7-K!5Tm!Ce~lR7B5m=v)glIhhv0iDDSV0=+12#i!U zbAKMfJo$b3I=xO7MG!&4;=T*uG?N5W3EUG7$`o`rR?vR^q$myFl1eh~F>nVdbk?{K zH&9IIam2C}U>_j`7aJn@JP{l=qJmTb7c9;5e431thoR5fa3BG?kPyxSOUET)Yhj-B z12H59WiE&+Qbj0m*q)EUO4Fr1a&kgXeSbcPiR+MVG3jGs)Oxn~4W|Sk8{FEE9~5W4 zz9pYbvusKrXV#fG%tw>sOQPWWuA~4 z#HvQ~XVZ&>cvsDnIt1%~rnGI_Y-^f;o2A-R#;gTol~J@8K-r&P)3^6&Y2W^ukbhp| zJQ>&$!lX29%ys3q78!R&-;#$$N?$nueM_vU0%E2@r^Dy4?l1|x*&#n~%O67F-+wUW zNV}D63ohHlpuOgPGkSG-b9Hllg~f8x!oi6iXYa%RDf{EA*W_$B{1fSUpO>S|S0N?K zRwC3*nF&KJf5^Ug+CoWDo^{|o@P8~gR^cvs4cpX8F4Fua!(wFp!Y?d@AV$w zlBen4nF?#ahqtU+}Fu+uIIp;pIsHdY$OvsSD{U)Py&D;oc7{ELR7hn}L>s-!bIWAr5p`F||?VpkO5 zY?to;W9|{zJ;Ke?#rJ{04cKgTdl|nf%ALKTduh zC9`@oP99<(s^Rhp-`*RYwq3OdAcc?j!%$sks_}um|F9%@rH+@!E%AYEJ@HI)yupD# zf5M>KRb{*@juN=9%ITx3)Wb_0|XQR z000O8#=6b3-~?y^1ID_|lS>Ah1ID_|lj#OF4#v98TrUDq^DF}Z0CESDEe9$C#=6au nS_c~fkpz?P1Q?T#2Mhzoy3LcG2O9$X1(Ul6CkB`X000009x~Hm diff --git a/tests/e2e/embed.spec.cjs b/tests/e2e/embed.spec.cjs index 47e55e5..bdc9c56 100644 --- a/tests/e2e/embed.spec.cjs +++ b/tests/e2e/embed.spec.cjs @@ -26,7 +26,7 @@ test('promotes whitelisted video + relative local PDF to inline parent players ( await page.goto('/tests/e2e/embed/parent.html'); const players = page.locator('.exe-embed-overlay iframe'); - await expect.poll(() => players.count(), { timeout: 15000 }).toBe(2); + await expect.poll(() => players.count(), { timeout: 15000 }).toBe(3); const srcs = await players.evaluateAll((els) => els.map((e) => e.src)); const hosts = srcs.map((s) => { @@ -35,6 +35,8 @@ test('promotes whitelisted video + relative local PDF to inline parent players ( // The video is rebuilt to the canonical nocookie URL (anchored, not a substring match). expect(srcs.some((s) => /^https:\/\/www\.youtube-nocookie\.com\/embed\/aqz-KE-bpKQ\b/.test(s))).toBe(true); + // A second provider (Dailymotion) is rebuilt to its canonical embed URL. + expect(srcs.some((s) => /^https:\/\/www\.dailymotion\.com\/embed\/video\/x8abc12$/.test(s))).toBe(true); // The relative local PDF is reported absolute and rendered (the relative-URL fix). expect(srcs.some((s) => /\/local\.pdf$/.test(s))).toBe(true); // The non-whitelisted host is never promoted (exact hostname check, not a substring). diff --git a/tests/e2e/embed/content.html b/tests/e2e/embed/content.html index 065d867..12607c9 100644 --- a/tests/e2e/embed/content.html +++ b/tests/e2e/embed/content.html @@ -8,11 +8,14 @@ + diff --git a/tests/e2e/embed/parent.html b/tests/e2e/embed/parent.html index 1cdfdb6..f2b2884 100644 --- a/tests/e2e/embed/parent.html +++ b/tests/e2e/embed/parent.html @@ -9,7 +9,9 @@ diff --git a/tests/js/exe_embed.test.js b/tests/js/exe_embed.test.js index d945333..122da17 100644 --- a/tests/js/exe_embed.test.js +++ b/tests/js/exe_embed.test.js @@ -26,6 +26,8 @@ const relay = window.exeEmbedRelay; const HOSTS = [ 'www.youtube.com', 'youtube.com', 'www.youtube-nocookie.com', 'youtube-nocookie.com', 'player.vimeo.com', 'vimeo.com', + 'www.dailymotion.com', 'dailymotion.com', 'geo.dailymotion.com', + 'mediateca.educa.madrid.org', ]; const WL = relay.buildWhitelist(HOSTS); const ORIGIN = window.location.origin; @@ -47,6 +49,27 @@ describe('exe_embed_relay validate() — videos', () => { .toEqual({ url: 'https://player.vimeo.com/video/76979871', kind: 'video' }); }); + it('rebuilds the canonical Dailymotion embed URL', () => { + expect(relay.validate('https://www.dailymotion.com/embed/video/x8abc12', CONTENT_SRC, WL)) + .toEqual({ url: 'https://www.dailymotion.com/embed/video/x8abc12', kind: 'video' }); + }); + + it('rejects a malformed Dailymotion path', () => { + expect(relay.validate('https://www.dailymotion.com/video/x8abc12', CONTENT_SRC, WL)).toBeNull(); + }); + + it('rebuilds the canonical EducaMadrid embed URL (adds /fs)', () => { + expect(relay.validate('https://mediateca.educa.madrid.org/video/u555bvi3bk5wsabh', CONTENT_SRC, WL)) + .toEqual({ url: 'https://mediateca.educa.madrid.org/video/u555bvi3bk5wsabh/fs', kind: 'video' }); + expect(relay.validate('https://mediateca.educa.madrid.org/video/u555bvi3bk5wsabh/fs', CONTENT_SRC, WL).url) + .toBe('https://mediateca.educa.madrid.org/video/u555bvi3bk5wsabh/fs'); + }); + + it('rejects an EducaMadrid look-alike host', () => { + expect(relay.validate('https://mediateca.educa.madrid.org.evil.com/video/u555bvi3bk5wsabh', CONTENT_SRC, WL)) + .toBeNull(); + }); + it('rejects a look-alike host (youtube.com.evil.com)', () => { expect(relay.validate('https://www.youtube.com.evil.com/embed/aqz-KE-bpKQ', CONTENT_SRC, WL)).toBeNull(); }); From a99997e7062c5bf1acf5e502b4aee8346fa2651d Mon Sep 17 00:00:00 2001 From: erseco Date: Sun, 14 Jun 2026 12:07:29 +0100 Subject: [PATCH 25/35] Fix lingering external embed when the eXe content pages to another view The in-iframe shim restarts its embed-id counter on every page, so after the content navigates (e.g. eXe multi-page next/prev), the new page's first embed reuses id exe-embed-1. The relay reused the existing player for that id and only repositioned it, never updating its src -- so the previous page's video (e.g. YouTube) lingered on the next page, stretched to the new box. Tag each player with the URL it renders (data-exe-embed-src) and, in sync(), replace the player when a reused id now maps to a different URL instead of repositioning the stale one. Add a regression test covering a reused id that navigates from YouTube to Vimeo. --- js/exe_embed_relay.js | 12 ++++++++++++ tests/js/exe_embed.test.js | 29 +++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/js/exe_embed_relay.js b/js/exe_embed_relay.js index b8a329d..54c0bf1 100644 --- a/js/exe_embed_relay.js +++ b/js/exe_embed_relay.js @@ -166,6 +166,10 @@ frame.setAttribute('referrerpolicy', 'no-referrer'); } frame.src = result.url; + // Tag the player with the URL it renders so sync() can detect when a reused + // embed id (the in-iframe shim restarts its counter per page) now points at a + // different URL and must be replaced rather than just repositioned. + frame.setAttribute('data-exe-embed-src', result.url); return frame; } @@ -238,6 +242,14 @@ } seen[embed.id] = true; var player = entry.players[embed.id]; + // After the content navigates, the shim reuses ids (exe-embed-1, ...) for + // the new page's embeds. If this id now renders a different URL, drop the + // stale player so the previous page's video does not linger here. + if (player && player.getAttribute('data-exe-embed-src') !== result.url) { + player.parentNode.removeChild(player); + delete entry.players[embed.id]; + player = null; + } if (!player) { player = makePlayer(result); entry.el.appendChild(player); diff --git a/tests/js/exe_embed.test.js b/tests/js/exe_embed.test.js index 122da17..a2bea9a 100644 --- a/tests/js/exe_embed.test.js +++ b/tests/js/exe_embed.test.js @@ -211,6 +211,35 @@ describe('exe_embed_relay createRelay() overlays players from messages', () => { expect(document.querySelectorAll('.exe-embed-overlay iframe').length).toBe(0); }); + it('replaces the player when a reused embed id navigates to a different URL (no lingering video)', () => { + const r = relay.createRelay({ whitelist: HOSTS }); + // Page 1: YouTube reported at id exe-embed-1. + r.onMessage({ + source: iframe.contentWindow, + data: { + type: 'exe-embed', + action: 'sync', + embeds: [{ id: 'exe-embed-1', url: 'https://www.youtube.com/embed/abc123', x: 0, y: 0, w: 480, h: 270 }], + }, + }); + expect(document.querySelector('.exe-embed-overlay iframe').src).toMatch(/youtube-nocookie\.com\/embed\/abc123$/); + + // Page 2: the in-iframe shim restarts its counter, so the Vimeo embed REUSES + // id exe-embed-1. The relay must swap the player, not just reposition it. + r.onMessage({ + source: iframe.contentWindow, + data: { + type: 'exe-embed', + action: 'sync', + embeds: [{ id: 'exe-embed-1', url: 'https://player.vimeo.com/video/12345', x: 0, y: 0, w: 425, h: 350 }], + }, + }); + const players = document.querySelectorAll('.exe-embed-overlay iframe'); + expect(players.length).toBe(1); + expect(players[0].src).toMatch(/player\.vimeo\.com\/video\/12345$/); + expect(players[0].src).not.toMatch(/youtube/); // The previous page's video must be gone. + }); + it('ignores a message whose source is not a known content iframe', () => { const r = relay.createRelay({ whitelist: HOSTS }); r.onMessage({ From 78f3189a48acfbc609cb1f36c8d1e752788f1d3e Mon Sep 17 00:00:00 2001 From: erseco Date: Sun, 14 Jun 2026 12:18:09 +0100 Subject: [PATCH 26/35] Document the embed providers/nav-fix and add the cross-repo drift check Update DEC-0061 with the 2026-06-14 work: the necessity re-validation against a real eXe export (machinery confirmed required on four axes, not assumed), the added Dailymotion + EducaMadrid providers, the sandbox-token alignment (allow-forms in the iframe attribute AND the response-level CSP sandbox directive), and the page-navigation lingering-embed fix. Add tools/check-embed-sync.mjs: a local maintenance helper that verifies the shim/relay logic + whitelist hosts + sandbox tokens stay in sync across the three embedders (mod canonical, wp/omeka mirrors). It is not a CI gate (no shared infra); the new header comments in the shim/relay point to it. --- ...-0061-embeds-externos-promote-to-parent.md | 46 +++++++ tools/check-embed-sync.mjs | 114 ++++++++++++++++++ 2 files changed, 160 insertions(+) create mode 100644 tools/check-embed-sync.mjs diff --git a/research/decisiones/adr/DEC-0061-embeds-externos-promote-to-parent.md b/research/decisiones/adr/DEC-0061-embeds-externos-promote-to-parent.md index 0ff5d39..c30ce74 100644 --- a/research/decisiones/adr/DEC-0061-embeds-externos-promote-to-parent.md +++ b/research/decisiones/adr/DEC-0061-embeds-externos-promote-to-parent.md @@ -138,6 +138,52 @@ Lista blanca por defecto: `www.youtube.com`, `youtube.com`, `www.youtube-nocooki - **Fixtures**: `research/fixtures/elpx/{youtube-embed,vimeo-embed,external-embeds-demo}.elpx` (el último con PDF remoto + PDF local empaquetado + imagen/vídeo remotos + un iframe no-whitelist). +## Actualización (2026-06-14): re-validación con export real, proveedores, tokens y fix de navegación + +### Re-validación de la NECESIDAD con un export real +Las fixtures sintéticas iniciales (`index.html` a mano) no eran representativas. Con un **export real +de eXeLearning** (`research/fixtures/elpx/remote-embeds.elpx`: `index.html` + `html/{vimeo, +mediatecamadrid,remote-pdf}-embed.html`) se repitió el experimento de control en mod (secure): +- shim **activado** → el vídeo de YouTube renderiza y reproduce; +- shim **desactivado** (control) → caja en **blanco**. + +Causa confirmada en 4 ejes: (1) **no es CSP** — `frame-src 'self' … https:` permite YouTube +(`player_iframe.php:171`), sin error CSP en consola; (2) es la **propagación del origen opaco** al +iframe anidado del player; (3) por especificación un contexto de navegación anidado no puede tener +*menos* restricciones que su padre; (4) lo único que lo "arregla" es `allow-same-origin`, que rompe el +aislamiento. **No hay alternativa más simple que preserve la seguridad: el overlay promote-to-parent es +la solución mínima.** (La duda razonable de erseco quedó así verificada, no asumida.) + +### Proveedores añadidos +**Dailymotion** (`www/geo.dailymotion.com`, embed `/embed/video/{id}`) y **EducaMadrid / Mediateca de +Madrid** (`mediateca.educa.madrid.org`, embed `/video/{id}/fs`), cada uno con validador por proveedor +que **reconstruye** la URL canónica (nunca pasa la del contenido tal cual). Verificado en vivo +(EducaMadrid renderiza inline en mod y omeka). + +### Alineación de tokens del sandbox (los 3) + fix de CSP oculto +Token seguro canónico = `allow-scripts allow-popups allow-forms` (los iDevices de eXe usan +formularios). wp/omeka **no tenían `allow-forms`** ni en el atributo del iframe ni — más grave — en la +**directiva CSP `sandbox`** del contenido servido (`class-content-proxy.php`, `ContentController.php`, +hardcodeadas). Sin `allow-forms` en la CSP, el envío de formularios se bloquea aunque el iframe lo +permita (el sandbox efectivo es la *intersección* de ambos). Corregido en los dos + tests. Legacy +alineado a `…allow-forms allow-popups-to-escape-sandbox`. + +### Bug de navegación entre páginas (reportado por erseco) + fix +Al paginar contenido eXe multi-página, el embed de la página anterior **quedaba pegado** en la nueva +(p.ej. YouTube persistía en la página de Vimeo, reescalado a su caja). Causa: el shim **reinicia su +contador de id por página**, así que el primer embed de cada página reusa `exe-embed-1`; el relay +reutilizaba el player existente y solo lo reposicionaba, **sin actualizar su `src`**. Reproducido en +vivo (la altura cambiaba a la de Vimeo pero el `src` seguía en YouTube). Fix: cada player se etiqueta +con `data-exe-embed-src`; en `sync()` se **reemplaza** cuando un id reusado apunta a otra URL. Test de +regresión Vitest + verificado en vivo en los 3 (mod: youtube→vimeo→mediateca→pdf cambian limpiamente; +wp y omeka, idem; el atributo del fix presente en el relevo servido). + +### Anti-drift +La lógica de shim/relay es idéntica en los 3 (solo cambia el envoltorio de export). mod es la **fuente +canónica** (`js/exe_embed_{shim,relay}.js`); wp/omeka llevan cabecera "MIRROR". `tools/check-embed-sync.mjs` +normaliza y compara la lógica + whitelist + tokens de las 3 copias y sale ≠0 si divergen (herramienta +de mantenimiento local, no gate CI cross-repo: no hay infra compartida). + ## Seguimiento - UI de admin para editar la lista blanca (hoy constante; configurable por filtro/ajuste como diff --git a/tools/check-embed-sync.mjs b/tools/check-embed-sync.mjs new file mode 100644 index 0000000..4510192 --- /dev/null +++ b/tools/check-embed-sync.mjs @@ -0,0 +1,114 @@ +#!/usr/bin/env node +// Maintenance helper: verify the external-embed shim/relay logic stays in sync across +// the three eXeLearning embedders. mod_exelearning is the CANONICAL source; wp-exelearning +// and omeka-s-exelearning mirror it -- only the export wrapper differs (mod is dual-export +// for Vitest; wp/omeka are auto-running IIFEs). This is NOT a CI gate (there is no shared +// infra across the three repos); it is a local check to run before/after touching the +// embedder. Exits non-zero when a copy has drifted (a required invariant is missing). +// +// Usage (mirror paths via flags or WP_EXE_DIR / OMEKA_EXE_DIR env vars): +// node tools/check-embed-sync.mjs --wp /path/to/wp-exelearning --omeka /path/to/omeka-s-exelearning +// With no mirror paths it only sanity-checks the canonical mod files and prints how to +// point it at the mirrors. + +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const MOD_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); + +/** Parse --wp / --omeka flags, falling back to env vars. */ +function resolveMirrors() { + const args = process.argv.slice(2); + const get = (flag) => { + const i = args.indexOf(flag); + return i !== -1 && args[i + 1] ? args[i + 1] : null; + }; + return { + wp: get('--wp') || process.env.WP_EXE_DIR || null, + omeka: get('--omeka') || process.env.OMEKA_EXE_DIR || null, + }; +} + +// Logic invariants every RELAY copy must contain (normalised: whitespace + quote style +// are ignored, so tabs-vs-spaces and the IIFE wrapper do not count as drift). +const RELAY_INVARIANTS = [ + 'youtube-nocookie.com/embed/', + 'player.vimeo.com/video/', + 'www.dailymotion.com/embed/video/', + 'mediateca.educa.madrid.org/video/', + 'data-exe-embed-src', // the page-navigation (id-reuse) fix + 'Math.min(embed.w,rect.width)', // overlay clamp (clickjacking defence) + 'Math.min(embed.h,rect.height)', + 'String(raw).indexOf("@")', // reject userinfo (evil.com@youtube.com) + 'event.source', // window-identity auth (opaque origin) +]; + +// Logic invariants every SHIM copy must contain. +const SHIM_INVARIANTS = [ + 'data-exe-embed-id', + 'data-exe-embed-url', + '__exeEmbedWhitelist', + '.pdf$', // the PDF detector (promote PDFs too) +]; + +// Host + token invariants every sandbox PHP must contain. +const PHP_INVARIANTS = [ + 'www.dailymotion.com', + 'geo.dailymotion.com', + 'mediateca.educa.madrid.org', + 'allow-scriptsallow-popupsallow-forms', // secure tokens (normalised, order matters) +]; + +const FILES = { + relay: { mod: 'js/exe_embed_relay.js', wp: 'assets/js/exe-embed-relay.js', omeka: 'asset/js/exe-embed-relay.js', invariants: RELAY_INVARIANTS }, + shim: { mod: 'js/exe_embed_shim.js', wp: 'assets/js/exe-embed-shim.js', omeka: 'asset/js/exe-embed-shim.js', invariants: SHIM_INVARIANTS }, + php: { mod: 'classes/local/ui/player_iframe.php', wp: 'includes/class-iframe-sandbox.php', omeka: 'src/Service/IframeSandbox.php', invariants: PHP_INVARIANTS }, +}; + +const norm = (s) => s.replace(/\s+/g, '').replace(/'/g, '"'); + +function check(label, absPath, invariants) { + if (!fs.existsSync(absPath)) { + return { label, path: absPath, missing: [''] }; + } + const body = norm(fs.readFileSync(absPath, 'utf8')); + const missing = invariants.filter((inv) => body.indexOf(norm(inv)) === -1); + return { label, path: absPath, missing }; +} + +function main() { + const mirrors = resolveMirrors(); + const roots = { mod: MOD_ROOT, wp: mirrors.wp, omeka: mirrors.omeka }; + const results = []; + + for (const [kind, spec] of Object.entries(FILES)) { + for (const repo of ['mod', 'wp', 'omeka']) { + if (!roots[repo]) { continue; } + results.push(check(`${repo}:${kind}`, path.join(roots[repo], spec[repo]), spec.invariants)); + } + } + + let drift = 0; + for (const r of results) { + if (r.missing.length) { + drift++; + console.error(`DRIFT ${r.label} (${r.path})`); + r.missing.forEach((m) => console.error(` missing: ${m}`)); + } else { + console.log(`ok ${r.label}`); + } + } + + if (!mirrors.wp || !mirrors.omeka) { + console.log('\nNote: pass --wp --omeka (or set WP_EXE_DIR / OMEKA_EXE_DIR)'); + console.log('to also check the wp-exelearning and omeka-s-exelearning mirrors.'); + } + if (drift) { + console.error(`\n${drift} file(s) drifted from the canonical embedder logic.`); + process.exit(1); + } + console.log('\nNo drift detected.'); +} + +main(); From 511f2b04cca8236c234bac9aafd8e6ff4c56bcfb Mon Sep 17 00:00:00 2001 From: erseco Date: Sun, 14 Jun 2026 14:10:44 +0100 Subject: [PATCH 27/35] Document the interactive-video iDevice limitation in secure mode (DEC-0061) The interactive-video iDevice with remote sources (YouTube/EducaMadrid) drives the YouTube IFrame Player API to pause and overlay timed questions; in the opaque sandbox the player cannot render in the iframe, and promoting it to the parent decouples it from the iDevice's control. A player-side control bridge was prototyped (video played, questions fired) but reverted: reconstructing the iDevice's cover/float/slide layout from the parent is fragile. Record the decision -- remote interactive-video needs legacy mode; local-file source works in secure -- and that the proper fix belongs upstream in the eXe iDevice (detect the opaque origin and degrade, or drive a parent relay itself). --- ...-0061-embeds-externos-promote-to-parent.md | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/research/decisiones/adr/DEC-0061-embeds-externos-promote-to-parent.md b/research/decisiones/adr/DEC-0061-embeds-externos-promote-to-parent.md index c30ce74..cc66326 100644 --- a/research/decisiones/adr/DEC-0061-embeds-externos-promote-to-parent.md +++ b/research/decisiones/adr/DEC-0061-embeds-externos-promote-to-parent.md @@ -184,6 +184,31 @@ canónica** (`js/exe_embed_{shim,relay}.js`); wp/omeka llevan cabecera "MIRROR". normaliza y compara la lógica + whitelist + tokens de las 3 copias y sale ≠0 si divergen (herramienta de mantenimiento local, no gate CI cross-repo: no hay infra compartida). +### Limitación conocida: iDevice de vídeo interactivo con fuente remota +El iDevice **vídeo interactivo** con fuentes remotas (YouTube/EducaMadrid) usa la **YouTube +IFrame Player API** (`new YT.Player` + `getCurrentTime`/`pauseVideo`/`seekTo`, 8 llamadas de +control en `idevices/interactive-video/interactive-video.js`) para pausar el vídeo y mostrar +preguntas cronometradas. En modo seguro esto **no funciona**: (1) en origen opaco el player +de YouTube no renderiza dentro del iframe; (2) promoverlo al padre (promote-to-parent) lo +**desacopla del control** del iDevice, que vive en el hijo opaco → sin pausa/seek/tiempo, las +preguntas no disparan. + +Se prototipó un **bridge de control YT** en el embedder (un `window.YT` falso en el hijo que +proxyea a un player real en el padre). Verificado en vivo: el vídeo **reproduce** y la +pregunta **aparece y se responde** en el segundo configurado. Pero **reconstruir el layout +del iDevice desde el padre — portada/`Inicio`, `float:left` de `#player` al activar +(`interactive-video.css` `.active #player{float:left}`), la posición de cada slide — es +frágil** (el vídeo aterriza mal y el solapamiento vídeo/pregunta no cuadra). **Decisión +(erseco): NO mantener el bridge en el embedder** (revertido, no commiteado). + +- **Estado en secure:** vídeo interactivo con fuente **remota** → usar **modo legacy**. Con + fuente **local** (archivo del paquete, HTML5 `