From 9425fd70c00edc6b6e1365bbee0aab6521c8ec41 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20N=C3=A9grier?= <d.negrier@thecodingmachine.com>
Date: Wed, 12 Jan 2022 17:22:41 +0100
Subject: [PATCH] Adding a new property to prevent script from being loaded in
 "modules" mode

Scripts in module mode need to be abide by the Same Origin Policy (CORS headers are used to load them)
This can cause issues on some setups.

This commit adds a new "scriptDisableModuleSupport" that can be used to disable the "modules" mode.

Closes #1721
---
 docs/maps/hosting.md                       |  5 ++
 docs/maps/menu.php                         |  8 +-
 docs/maps/scripting-internals.md           | 62 +++++++++++++++
 docs/maps/scripting.md                     | 19 ++++-
 front/dist/iframe.html                     |  5 +-
 front/src/Api/IframeListener.ts            | 12 ++-
 front/src/Phaser/Game/GameMapProperties.ts |  1 +
 front/src/Phaser/Game/GameScene.ts         |  5 +-
 maps/tests/Modules/module.js               |  1 +
 maps/tests/Modules/script.js               |  3 +
 maps/tests/Modules/with_modules.json       | 88 ++++++++++++++++++++
 maps/tests/Modules/without_modules.json    | 93 ++++++++++++++++++++++
 maps/tests/index.html                      | 16 ++++
 tests/tests/modules.ts                     | 44 ++++++++++
 14 files changed, 353 insertions(+), 9 deletions(-)
 create mode 100644 docs/maps/scripting-internals.md
 create mode 100644 maps/tests/Modules/module.js
 create mode 100644 maps/tests/Modules/script.js
 create mode 100644 maps/tests/Modules/with_modules.json
 create mode 100644 maps/tests/Modules/without_modules.json
 create mode 100644 tests/tests/modules.ts

diff --git a/docs/maps/hosting.md b/docs/maps/hosting.md
index cd3d310d..b0d8bc22 100644
--- a/docs/maps/hosting.md
+++ b/docs/maps/hosting.md
@@ -12,6 +12,11 @@ If you decide to host your maps on your own webserver, you must **configure CORS
 
 CORS headers ([Cross Origin Resource Sharing](https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS)) are useful when a website want to make some resources accessible to another website. This is exactly what we want to do. We want the map you are designing to be accessible from the WorkAdventure domain (`play.workadventu.re`).
 
+{.alert.alert-warning}
+If you are using the "scripting API", only allowing the `play.workadventu.re` will not be enough. You will need to allow `*`
+as a domain in order to be able to load scripts. If for some reason, you cannot or do not want to allow `*` as a domain, please
+read the [scripting internals](scripting-internals.md) guide for alternatives.
+
 ### Enabling CORS for Apache
 
 In order to enable CORS in your Apache configuration, you will need to ensure the `headers` module is enabled.
diff --git a/docs/maps/menu.php b/docs/maps/menu.php
index 10a2f4c5..b8d0e638 100644
--- a/docs/maps/menu.php
+++ b/docs/maps/menu.php
@@ -149,7 +149,13 @@ return [
                     ],
                 ]
             ],
-            $extraUtilsMenu
+            $extraUtilsMenu,
+            [
+                'title' => 'Scripting internals',
+                'url' => '/map-building/scripting-internals.md',
+                'markdown' => 'maps.scripting-internals',
+                'editUrl' => 'https://github.com/thecodingmachine/workadventure/edit/develop/docs/maps/scripting-internals.md',
+            ],
         ]
     ],
     [
diff --git a/docs/maps/scripting-internals.md b/docs/maps/scripting-internals.md
new file mode 100644
index 00000000..65bd9850
--- /dev/null
+++ b/docs/maps/scripting-internals.md
@@ -0,0 +1,62 @@
+{.section-title.accent.text-primary}
+# Scripting internals
+
+Internally, scripts are always loaded inside `iframes`.
+
+You can load a script:
+
+1. Using the [`script` property in your map properties](scripting.md#adding-a-script-in-the-map)
+2. or from an iframe [opened as a co-website](scripting.md#adding-a-script-in-an-iframe) or [embedded in the map](website-in-map.md#allowing-the-scripting-api-in-your-iframe)
+
+## Script restrictions
+
+If you load a script using the `script` property in your map properties (solution 1), you need to understand that
+WorkAdventure will generate an iframe, and will load the script inside this iframe.
+
+Things you should know:
+
+{.alert.alert-warning}
+The [iframe is sandboxed](https://blog.dareboost.com/en/2015/07/securing-iframe-sandbox-attribute/)
+
+This means that the iframe is generated with:
+
+```
+<iframe src="..." sandbox="allow-scripts allow-top-navigation-by-user-activation" />
+```
+
+Such an iframe has restrictions. In particular, it does NOT have an origin.
+Because it has no origin, XHR requests cannot be made from those scripts.
+
+If you absolutely need to make a request to an external server from your script, you can:
+
+- use websockets (that are not subject to CORS restrictions)
+- or load the script inside an embedded iframe (that you hide somewhere on the map)
+
+## Script, modules and CORS issues
+
+If you load a script using the `script` property in your map properties (solution 1), scripts are loaded by default with the
+`type="module"` attribute. Because those scripts are [loaded as modules](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Modules#applying_the_module_to_your_html),
+they need to abide by the same-origin policy, so they are using CORS.
+
+But because the iframe is sandboxed, the script does not have an origin. Therefore, the webserver hosting your script
+will need to allow **all** origins with:
+
+```
+Access-Control-Allow-Origin: *
+```
+or alternatively:
+```
+Access-Control-Allow-Origin: null
+```
+
+This should not be a security concern if your website is only hosting static files. However, in the event the website
+hosting the script is also hosting dynamic content, please be careful before allowing those headers on a site-wide basis.
+
+If you cannot or do not want to allow CORS to all domains, there is an alternative: you can remove the `type="module"` attribute
+from the script. The script will not be able to load modules anymore but will not be bound to the same origin policy anymore
+so the `Access-Control-Allow-Origin` header is not needed anymore for this script.
+
+To remove the `type="module"` attribute from the script, in your map properties, next to the `script` attribute,
+add a `scriptDisableModuleSupport` boolean property and set this property to "checked".
+
+![](images/script-disable-modules-support.png)
diff --git a/docs/maps/scripting.md b/docs/maps/scripting.md
index 6da3ddbf..d50f63d4 100644
--- a/docs/maps/scripting.md
+++ b/docs/maps/scripting.md
@@ -1,6 +1,3 @@
-{.alert.alert-danger style="width:80%"}
-This feature is "_experimental_". We may apply changes in the near future to the way it works when we gather some feedback.
-
 {.section-title.accent.text-primary}
 # Scripting WorkAdventure maps
 
@@ -62,6 +59,22 @@ The `WA` objects contains a number of useful methods enabling you to interact wi
 
 The message should be displayed in the chat history as soon as you enter the room.
 
+{.alert.alert-warning}
+Internally, scripts are running inside a [sandboxed iframe](https://blog.dareboost.com/en/2015/07/securing-iframe-sandbox-attribute/).
+Furthermore, the script itself is loaded as module with `<script src="" type="module">`. Scripts loaded as module must enforce CORS.
+But the iframe itself does not have any origin, because it is sandboxed. As a result, for the script to be loaded correctly,
+you will need to allow ALL origins using this header:
+```
+Access-Control-Allow-Origin: *
+```
+or alternatively:
+```
+Access-Control-Allow-Origin: null
+```
+
+Because the script is sandboxed, a number of restrictions apply. If you want a discussion on how to overcome them,
+check out the ["scripting internals" documentation](scripting-internals.md).
+
 ## Adding a script in an iFrame
 
 In WorkAdventure, you can easily [open an iFrame using the `openWebsite` property on a layer](special-zones). However, by default, the iFrame is not allowed to communicate with WorkAdventure.
diff --git a/front/dist/iframe.html b/front/dist/iframe.html
index 02dc0fd8..14e65b6f 100644
--- a/front/dist/iframe.html
+++ b/front/dist/iframe.html
@@ -11,7 +11,10 @@
         const scriptUrl = urlParams.get('script');
         const script = document.createElement('script');
         script.src = scriptUrl;
-        script.type = "module";
+
+        if (urlParams.get('moduleMode') === 'true') {
+            script.type = "module";
+        }
         document.head.append(script);
     </script>
     </head>
diff --git a/front/src/Api/IframeListener.ts b/front/src/Api/IframeListener.ts
index 216a9510..85e1373b 100644
--- a/front/src/Api/IframeListener.ts
+++ b/front/src/Api/IframeListener.ts
@@ -274,7 +274,7 @@ class IframeListener {
         this.iframes.delete(iframe);
     }
 
-    registerScript(scriptUrl: string): Promise<void> {
+    registerScript(scriptUrl: string, enableModuleMode: boolean = true): Promise<void> {
         return new Promise<void>((resolve, reject) => {
             console.info("Loading map related script at ", scriptUrl);
 
@@ -283,7 +283,11 @@ class IframeListener {
                 const iframe = document.createElement("iframe");
                 iframe.id = IframeListener.getIFrameId(scriptUrl);
                 iframe.style.display = "none";
-                iframe.src = "/iframe.html?script=" + encodeURIComponent(scriptUrl);
+                iframe.src =
+                    "/iframe.html?script=" +
+                    encodeURIComponent(scriptUrl) +
+                    "&moduleMode=" +
+                    (enableModuleMode ? "true" : "false");
 
                 // We are putting a sandbox on this script because it will run in the same domain as the main website.
                 iframe.sandbox.add("allow-scripts");
@@ -318,7 +322,9 @@ class IframeListener {
                     "//" +
                     window.location.host +
                     '/iframe_api.js" ></script>\n' +
-                    '<script type="module" src="' +
+                    "<script " +
+                    (enableModuleMode ? 'type="module" ' : "") +
+                    'src="' +
                     scriptUrl +
                     '" ></script>\n' +
                     "<title></title>\n" +
diff --git a/front/src/Phaser/Game/GameMapProperties.ts b/front/src/Phaser/Game/GameMapProperties.ts
index 65ce6ab8..b77bd02b 100644
--- a/front/src/Phaser/Game/GameMapProperties.ts
+++ b/front/src/Phaser/Game/GameMapProperties.ts
@@ -28,6 +28,7 @@ export enum GameMapProperties {
     PLAY_AUDIO_LOOP = "playAudioLoop",
     READABLE_BY = "readableBy",
     SCRIPT = "script",
+    SCRIPT_DISABLE_MODULE_SUPPORT = "scriptDisableModuleSupport",
     SILENT = "silent",
     START = "start",
     START_LAYER = "startLayer",
diff --git a/front/src/Phaser/Game/GameScene.ts b/front/src/Phaser/Game/GameScene.ts
index 740de4c1..6b449948 100644
--- a/front/src/Phaser/Game/GameScene.ts
+++ b/front/src/Phaser/Game/GameScene.ts
@@ -597,9 +597,12 @@ export class GameScene extends DirtyScene {
         this.createPromiseResolve();
         // Now, let's load the script, if any
         const scripts = this.getScriptUrls(this.mapFile);
+        const disableModuleMode = this.getProperty(this.mapFile, GameMapProperties.SCRIPT_DISABLE_MODULE_SUPPORT) as
+            | boolean
+            | undefined;
         const scriptPromises = [];
         for (const script of scripts) {
-            scriptPromises.push(iframeListener.registerScript(script));
+            scriptPromises.push(iframeListener.registerScript(script, !disableModuleMode));
         }
 
         this.userInputManager.spaceEvent(() => {
diff --git a/maps/tests/Modules/module.js b/maps/tests/Modules/module.js
new file mode 100644
index 00000000..d407b060
--- /dev/null
+++ b/maps/tests/Modules/module.js
@@ -0,0 +1 @@
+export const foo = "bar";
diff --git a/maps/tests/Modules/script.js b/maps/tests/Modules/script.js
new file mode 100644
index 00000000..578a7148
--- /dev/null
+++ b/maps/tests/Modules/script.js
@@ -0,0 +1,3 @@
+import { foo } from './module.js';
+
+console.log('Successfully loaded module: foo = ', foo);
diff --git a/maps/tests/Modules/with_modules.json b/maps/tests/Modules/with_modules.json
new file mode 100644
index 00000000..2dd40427
--- /dev/null
+++ b/maps/tests/Modules/with_modules.json
@@ -0,0 +1,88 @@
+{ "compressionlevel":-1,
+ "height":10,
+ "infinite":false,
+ "layers":[
+        {
+         "data":[1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1],
+         "height":10,
+         "id":1,
+         "name":"floor",
+         "opacity":1,
+         "type":"tilelayer",
+         "visible":true,
+         "width":10,
+         "x":0,
+         "y":0
+        }, 
+        {
+         "data":[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 12, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
+         "height":10,
+         "id":2,
+         "name":"start",
+         "opacity":1,
+         "type":"tilelayer",
+         "visible":true,
+         "width":10,
+         "x":0,
+         "y":0
+        }, 
+        {
+         "draworder":"topdown",
+         "id":3,
+         "name":"floorLayer",
+         "objects":[
+                {
+                 "height":304.037037037037,
+                 "id":3,
+                 "name":"",
+                 "rotation":0,
+                 "text":
+                    {
+                     "fontfamily":"Sans Serif",
+                     "pixelsize":11,
+                     "text":"Test:\nOpen the console.\nThe script loaded loads modules. This should work.\n\nYou should see in the console:\n\nSuccessfully loaded module: foo = \"bar\"",
+                     "wrap":true
+                    },
+                 "type":"",
+                 "visible":true,
+                 "width":252.4375,
+                 "x":2.78125,
+                 "y":2.5
+                }],
+         "opacity":1,
+         "type":"objectgroup",
+         "visible":true,
+         "x":0,
+         "y":0
+        }],
+ "nextlayerid":9,
+ "nextobjectid":11,
+ "orientation":"orthogonal",
+ "properties":[
+        {
+         "name":"script",
+         "type":"string",
+         "value":"script.js"
+        }],
+ "renderorder":"right-down",
+ "tiledversion":"2021.03.23",
+ "tileheight":32,
+ "tilesets":[
+        {
+         "columns":11,
+         "firstgid":1,
+         "image":"..\/tileset1.png",
+         "imageheight":352,
+         "imagewidth":352,
+         "margin":0,
+         "name":"tileset1",
+         "spacing":0,
+         "tilecount":121,
+         "tileheight":32,
+         "tilewidth":32
+        }],
+ "tilewidth":32,
+ "type":"map",
+ "version":1.5,
+ "width":10
+}
\ No newline at end of file
diff --git a/maps/tests/Modules/without_modules.json b/maps/tests/Modules/without_modules.json
new file mode 100644
index 00000000..4b64af57
--- /dev/null
+++ b/maps/tests/Modules/without_modules.json
@@ -0,0 +1,93 @@
+{ "compressionlevel":-1,
+ "height":10,
+ "infinite":false,
+ "layers":[
+        {
+         "data":[1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1],
+         "height":10,
+         "id":1,
+         "name":"floor",
+         "opacity":1,
+         "type":"tilelayer",
+         "visible":true,
+         "width":10,
+         "x":0,
+         "y":0
+        }, 
+        {
+         "data":[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 12, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
+         "height":10,
+         "id":2,
+         "name":"start",
+         "opacity":1,
+         "type":"tilelayer",
+         "visible":true,
+         "width":10,
+         "x":0,
+         "y":0
+        }, 
+        {
+         "draworder":"topdown",
+         "id":3,
+         "name":"floorLayer",
+         "objects":[
+                {
+                 "height":304.037037037037,
+                 "id":3,
+                 "name":"",
+                 "rotation":0,
+                 "text":
+                    {
+                     "fontfamily":"Sans Serif",
+                     "pixelsize":11,
+                     "text":"Test:\nOpen the console.\nThe script loaded loads modules. This should fail because we disallow modules.\n\nYou should see in the console this error message:\n\nUncaught SyntaxError: Cannot use import statement outside a module",
+                     "wrap":true
+                    },
+                 "type":"",
+                 "visible":true,
+                 "width":252.4375,
+                 "x":2.78125,
+                 "y":2.5
+                }],
+         "opacity":1,
+         "type":"objectgroup",
+         "visible":true,
+         "x":0,
+         "y":0
+        }],
+ "nextlayerid":9,
+ "nextobjectid":11,
+ "orientation":"orthogonal",
+ "properties":[
+        {
+         "name":"script",
+         "type":"string",
+         "value":"script.js"
+        }, 
+        {
+         "name":"scriptDisableModuleSupport",
+         "type":"bool",
+         "value":true
+        }],
+ "renderorder":"right-down",
+ "tiledversion":"2021.03.23",
+ "tileheight":32,
+ "tilesets":[
+        {
+         "columns":11,
+         "firstgid":1,
+         "image":"..\/tileset1.png",
+         "imageheight":352,
+         "imagewidth":352,
+         "margin":0,
+         "name":"tileset1",
+         "spacing":0,
+         "tilecount":121,
+         "tileheight":32,
+         "tilewidth":32
+        }],
+ "tilewidth":32,
+ "type":"map",
+ "version":1.5,
+ "width":10
+}
\ No newline at end of file
diff --git a/maps/tests/index.html b/maps/tests/index.html
index 573c02f7..a3b937e3 100644
--- a/maps/tests/index.html
+++ b/maps/tests/index.html
@@ -275,6 +275,22 @@
                         <a href="#" class="testLink" data-testmap="Outline/outline.json" target="_blank">Testing scripting API for outline on players</a>
                     </td>
                 </tr>
+                <tr>
+                    <td>
+                        <input type="radio" name="test-js-modules"> Success <input type="radio" name="test-js-modules"> Failure <input type="radio" name="test-js-modules" checked> Pending
+                    </td>
+                    <td>
+                        <a href="#" class="testLink" data-testmap="Modules/with_modules.json" target="_blank">Testing scripts with modules</a>
+                    </td>
+                </tr>
+                <tr>
+                    <td>
+                        <input type="radio" name="test-js-no-modules"> Success <input type="radio" name="test-js-no-modules"> Failure <input type="radio" name="test-js-no-modules" checked> Pending
+                    </td>
+                    <td>
+                        <a href="#" class="testLink" data-testmap="Modules/without_modules.json" target="_blank">Testing scripts with modules mode disabled</a>
+                    </td>
+                </tr>
             </table>
             <h2>CoWebsite</h2>
             <table class="table">
diff --git a/tests/tests/modules.ts b/tests/tests/modules.ts
new file mode 100644
index 00000000..86f7a374
--- /dev/null
+++ b/tests/tests/modules.ts
@@ -0,0 +1,44 @@
+import {assertLogMessage} from "./utils/log";
+
+const fs = require('fs');
+const Docker = require('dockerode');
+import { Selector } from 'testcafe';
+import {login} from "./utils/roles";
+import {
+    findContainer,
+    rebootBack,
+    rebootPusher,
+    resetRedis,
+    rebootTraefik,
+    startContainer,
+    stopContainer, stopRedis, startRedis
+} from "./utils/containers";
+import {getBackDump, getPusherDump} from "./utils/debug";
+
+fixture `Modules`
+    .page `http://play.workadventure.localhost/_/global/maps.workadventure.localhost/tests/Modules/with_modules.json`;
+
+test("Test that module loading works out of the box", async (t: TestController) => {
+
+    await login(t, 'http://play.workadventure.localhost/_/global/maps.workadventure.localhost/tests/Modules/with_modules.json');
+
+
+    await assertLogMessage(t, 'Successfully loaded module: foo =  bar');
+
+    t.ctx.passed = true;
+}).after(async t => {
+    if (!t.ctx.passed) {
+        console.log("Test 'Test that module loading works out of the box' failed. Browser logs:")
+        try {
+            console.log(await t.getBrowserConsoleMessages());
+        } catch (e) {
+            console.error('Error while fetching browser logs (maybe linked to a closed iframe?)', e);
+            try {
+                console.log('Logs from main window:');
+                console.log(await t.switchToMainWindow().getBrowserConsoleMessages());
+            } catch (e) {
+                console.error('Unable to retrieve logs', e);
+            }
+        }
+    }
+});