From 98f946c871bce68c19929e257571124312715b48 Mon Sep 17 00:00:00 2001 From: jf-cbd Date: Fri, 7 Jun 2024 14:18:14 +0200 Subject: [PATCH 1/2] =?UTF-8?q?=20N=C2=B07124=20-=20[SECU]=20Cross-Site=20?= =?UTF-8?q?Request=20Forgery=20(CSRF)=20in=20several=20iTop=20pages=20(fin?= =?UTF-8?q?alize=20implementation)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../portal/templates/layout.html.twig | 1 + js/ajax_hook.js | 5 +++++ js/ckeditor.feeds.js | 6 +----- js/ckeditor.handler.js | 7 ++----- js/icon_select.js | 6 +----- js/pages/backoffice/on-ready.js | 5 ----- js/utils.js | 17 +++++++++++++++++ pages/ajax.render.php | 4 ++-- sources/Application/WebPage/NiceWebPage.php | 1 + .../Controller/OAuth/OAuthLandingController.php | 1 + 10 files changed, 31 insertions(+), 22 deletions(-) create mode 100644 js/ajax_hook.js diff --git a/datamodels/2.x/itop-portal-base/portal/templates/layout.html.twig b/datamodels/2.x/itop-portal-base/portal/templates/layout.html.twig index 2b1780082..a584b168e 100644 --- a/datamodels/2.x/itop-portal-base/portal/templates/layout.html.twig +++ b/datamodels/2.x/itop-portal-base/portal/templates/layout.html.twig @@ -87,6 +87,7 @@ {% block pPageScripts %} + {% if is_development_environment() %} {% else %} diff --git a/js/ajax_hook.js b/js/ajax_hook.js new file mode 100644 index 000000000..aa91d51e2 --- /dev/null +++ b/js/ajax_hook.js @@ -0,0 +1,5 @@ +// add X-Combodo-Ajax for all request (just after jaquery is loaded) +// mandatory for ajax requests with JQuery (CSRF protection) +$(document).ajaxSend(function (event, jqxhr, options) { + jqxhr.setRequestHeader('X-Combodo-Ajax', 'true'); +}); \ No newline at end of file diff --git a/js/ckeditor.feeds.js b/js/ckeditor.feeds.js index b06279602..0eed695d4 100644 --- a/js/ckeditor.feeds.js +++ b/js/ckeditor.feeds.js @@ -16,11 +16,7 @@ const CombodoCKEditorFeeds = { return async function(queryText) { return new Promise( resolve => { setTimeout( () => { - fetch(options.url + queryText, { - headers: { - 'X-Combodo-Ajax': true - } - }) + CombodoHTTP.Fetch(options.url + queryText) .then(response => { return response.json(); }) diff --git a/js/ckeditor.handler.js b/js/ckeditor.handler.js index 390cd29a5..7f7cfc836 100644 --- a/js/ckeditor.handler.js +++ b/js/ckeditor.handler.js @@ -112,12 +112,9 @@ const CombodoCKEditorHandler = { const formData = new FormData(); formData.append('upload', file); - fetch(uploadUrl, { + CombodoHTTP.Fetch(uploadUrl, { method: 'POST', - body: formData, - headers: { - 'X-Combodo-Ajax': true - }, + body: formData }) .then(response => response.json()) .then(responseData => { diff --git a/js/icon_select.js b/js/icon_select.js index 5eaa590b7..a9e6bfcfb 100644 --- a/js/icon_select.js +++ b/js/icon_select.js @@ -247,12 +247,8 @@ $(function() var file = event.target.files[0]; var formData = new FormData(); formData.append('file', file); - - fetch(this.options.post_upload_to, { + CombodoHTTP.Fetch(this.options.post_upload_to, { method: 'POST', - headers: { - 'X-Combodo-Ajax': true - }, body: formData }) .then(response => { diff --git a/js/pages/backoffice/on-ready.js b/js/pages/backoffice/on-ready.js index e477de897..1d7f54192 100644 --- a/js/pages/backoffice/on-ready.js +++ b/js/pages/backoffice/on-ready.js @@ -8,11 +8,6 @@ */ $(document).ready(function () { - // AJAX calls handling - // - Custom headers - $(document).ajaxSend(function (event, jqxhr, options) { - jqxhr.setRequestHeader('X-Combodo-Ajax', 'true'); - }); // - Error messages regarding the error code $(document).ajaxError(function (event, jqxhr, options) { // User is not logged in diff --git a/js/utils.js b/js/utils.js index 5bb9fc24f..4d51b4ee0 100644 --- a/js/utils.js +++ b/js/utils.js @@ -1296,6 +1296,23 @@ const CombodoInlineImage = { } }; +/** + * Abstract Fetch API wrapper to manage AJAX requests in iTop. + */ +const CombodoHTTP = { + /** + * @param {string} sUrl URL to fetch + * @param {Object} oOptions Fetch options + * @return {Promise} + */ + Fetch: function(sUrl, oOptions) { + oOptions = oOptions || {}; + oOptions.headers = oOptions.headers || {}; + oOptions.headers['X-Combodo-Ajax'] = true; + return fetch(sUrl, oOptions); + } +} + /** * Abstract wrapper to manage modal dialogs in iTop. * Implementations for the various GUIs may vary but APIs are the same. diff --git a/pages/ajax.render.php b/pages/ajax.render.php index 4a194a869..89135c9db 100644 --- a/pages/ajax.render.php +++ b/pages/ajax.render.php @@ -29,12 +29,12 @@ require_once('../approot.inc.php'); // check if header contains X-Combodo-Ajax for POST request (CSRF protection for ajax calls) -/*if (!isset($_SERVER['HTTP_X_COMBODO_AJAX']) && $_SERVER['REQUEST_METHOD'] === 'POST') { +if (!isset($_SERVER['HTTP_X_COMBODO_AJAX']) && $_SERVER['REQUEST_METHOD'] === 'POST') { $sReferer = utils::HtmlEntities($_SERVER['HTTP_REFERER']); IssueLog::Error("Unprotected ajax call from: $sReferer", 'SECURITY'); header('HTTP/1.1 401 Unauthorized'); die('Unauthorized access. Please see https://www.itophub.io/wiki/page?id=3_2_0:release:developer#checking_for_the_presence_of_specific_header_in_the_post_to_enhance_protection_against_csrf_attacks'); -}*/ +} function LogErrorMessage($sMsgPrefix, $aContextInfo) { diff --git a/sources/Application/WebPage/NiceWebPage.php b/sources/Application/WebPage/NiceWebPage.php index e8e0051ef..4e105a5dc 100644 --- a/sources/Application/WebPage/NiceWebPage.php +++ b/sources/Application/WebPage/NiceWebPage.php @@ -153,6 +153,7 @@ JS // Used throughout the app. $this->LinkScriptFromAppRoot('node_modules/jquery/dist/jquery.min.js'); + $this->LinkScriptFromAppRoot('js/ajax_hook.js'); $this->LinkScriptFromAppRoot('js/jquery.blockUI.js'); if (utils::IsDevelopmentEnvironment()) // Needed since many other plugins still rely on oldies like $.browser { diff --git a/sources/Controller/OAuth/OAuthLandingController.php b/sources/Controller/OAuth/OAuthLandingController.php index fd0e03807..a6429738c 100644 --- a/sources/Controller/OAuth/OAuthLandingController.php +++ b/sources/Controller/OAuth/OAuthLandingController.php @@ -10,6 +10,7 @@ class OAuthLandingController extends Controller public function OperationLanding() { $this->AddLinkedScript(utils::GetAbsoluteUrlAppRoot().'node_modules/jquery/dist/jquery.min.js'); + $this->AddLinkedScript(utils::GetAbsoluteUrlAppRoot().'js/ajax_hook.js'); $this->DisplayPage([], null, static::ENUM_PAGE_TYPE_BASIC_HTML); } } \ No newline at end of file From ab9df22e76bbb729c01d007abd9a041ae0674578 Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Tue, 2 Jul 2024 16:55:16 +0200 Subject: [PATCH 2/2] :memo: Reinforce the documentation of WebPage JS inclusion API --- sources/Application/WebPage/WebPage.php | 87 +++++++++++++++++++++---- 1 file changed, 74 insertions(+), 13 deletions(-) diff --git a/sources/Application/WebPage/WebPage.php b/sources/Application/WebPage/WebPage.php index ba442ff85..b984cd597 100644 --- a/sources/Application/WebPage/WebPage.php +++ b/sources/Application/WebPage/WebPage.php @@ -47,6 +47,17 @@ use const MODULESROOT; * $oPage->p("Hello World !"); * $oPage->output(); * ``` + *

+ *

+ * JS scripts can be added. They will be executed in sequence depending on the API used to inject them: + *

    + *
  1. add_early_script()
    2. + *
  2. LinkScriptFromAppRoot(), LinkScriptFromURI(), LinkScriptFromModule()
    3. + *
  3. add_script()
    4. + *
  4. add_init_script() - DOMContentLoaded
    5. + *
  5. add_ready_script() - DOMContentLoaded + 50 ms
    6. + *
+ *

*/ class WebPage implements Page { @@ -660,12 +671,21 @@ class WebPage implements Page } /** - * Add some Javascript to the header of the page, therefore executed first, BEFORE the DOM interpretation. + * Add some Javascript to the header of the page + * + * The provided JS code will be executed at step 1 of the JS execution chain: + * [early script] ==> linked script ==> script ==> init script ==> ready script + * * /!\ Keep in mind that no external JS files (eg. jQuery) will be loaded yet. * - * @uses WebPage::$a_a_early_scripts + * @api-advanced + * @see static::add_script, static::add_init_script, static::add_ready_script + * @see static::LinkScriptFromAppRoot, static::LinkScriptFromModule, static::LinkScriptFromURI + * * @param string $s_script + * * @since 3.0.0 + * @uses WebPage::$a_early_scripts */ public function add_early_script($s_script) { @@ -701,8 +721,16 @@ class WebPage implements Page /** * Add some Javascript to be executed immediately without waiting for the DOM to be ready * - * @uses WebPage::$a_scripts + * The provided JS code will be executed at step 3 of the JS execution chain: + * early script ==> linked script ==> [script] ==> init script ==> ready script + * + * @api-advanced + * @see static::add_early_script, static::add_init_script, static::add_ready_script + * @see static::LinkScriptFromAppRoot, static::LinkScriptFromURI, static::LinkScriptFromModule + * * @param string $s_script + * + * @uses WebPage::$a_scripts * @since 3.0.0 These scripts are put at the end of the tag instead of the end of the tag, {@see static::add_early_script} to add script there */ public function add_script($s_script) @@ -737,12 +765,19 @@ class WebPage implements Page } /** - * Adds a script to be executed when the DOM is ready (typical JQuery use), right before add_ready_script + * Add a script to be executed when the DOM is ready (typical JQuery use) + * + * The provided JS code will be executed at step 4 of the JS execution chain: + * early script ==> linked script ==> script ==> [init script] ==> ready script + * + * @api-advanced + * @see static::add_early_script, static::add_script, static::add_ready_script + * @see static::LinkScriptFromAppRoot, static::LinkScriptFromURI, static::LinkScriptFromModule * - * @uses WebPage::$a_init_scripts * @param string $sScript * * @return void + *@uses WebPage::$a_init_scripts */ public function add_init_script($sScript) { @@ -776,10 +811,18 @@ class WebPage implements Page } /** - * Add some Javascript to be executed once the DOM is ready, slightly after the "init scripts" + * Add some Javascript to be executed once the DOM is ready, slightly (50 ms) after the "init scripts" + * + * The provided JS code will be executed at step 5 of the JS execution chain: + * early script ==> linked script ==> script ==> init script ==> [ready script] + * + * @api-advanced + * @see static::add_early_script, static::add_script, static::add_init_script + * @see static::LinkScriptFromAppRoot, static::LinkScriptFromURI, static::LinkScriptFromModule + * + * @param $sScript * * @uses WebPage::$a_ready_scripts - * @param $sScript */ public function add_ready_script($sScript) { @@ -886,14 +929,20 @@ class WebPage implements Page } /** - * Use to link JS files from the iTop package (e.g. `/js/*`) + * Use to include JS files from the iTop package (e.g. `/js/*`) + * + * The provided JS code will be executed at step 2 of the JS execution chain: + * early script ==> [linked script] ==> script ==> init script ==> ready script + * + * @api-advanced + * @see static::add_early_script, static::add_script, static::add_init_script, static::add_ready_script + * @see static::LinkScriptFromURI, static::LinkScriptFromModule * * @param string $sFileRelPath Rel. path from iTop app. root of the JS file to link (e.g. `js/utils.js`) * * @return void * @throws \Exception * @since 3.2.0 N°7315 - * @api */ public function LinkScriptFromAppRoot(string $sFileRelPath): void { @@ -901,14 +950,20 @@ class WebPage implements Page } /** - * Use to link JS files from any module + * Use to include JS files from any module + * + * The provided JS code will be executed at step 2 of the JS execution chain: + * early script ==> [linked script] ==> script ==> init script ==> ready script + * + * @api-advanced + * @see static::add_early_script, static::add_script, static::add_init_script, static::add_ready_script + * @see static::LinkScriptFromAppRoot, static::LinkScriptFromURI * * @param string $sFileRelPath Rel. path from current environment (e.g. `/env-production/`) of the JS file to link (e.g. `some-module/asset/js/some-file.js`) * * @return void * @throws \Exception * @since 3.2.0 N°7315 - * @api */ public function LinkScriptFromModule(string $sFileRelPath): void { @@ -916,7 +971,14 @@ class WebPage implements Page } /** - * Use to link JS files from any URI, typically an external server + * Use to include JS files from any URI, typically an external server + * + * The provided JS code will be executed at step 2 of the JS execution chain: + * early script ==> [linked script] ==> script ==> init script ==> ready script + * + * @api-advanced + * @see static::add_early_script, static::add_script, static::add_init_script, static::add_ready_script + * @see static::LinkScriptFromAppRoot, static::LinkScriptFromModule * * @param string $sFileAbsURI Abs. URI of the JS file to link (e.g. `https://external.server.com/some-file.js`) * Note: Any non-absolute URI will be ignored. @@ -924,7 +986,6 @@ class WebPage implements Page * @return void * @throws \Exception * @since 3.2.0 N°7315 - * @api */ public function LinkScriptFromURI(string $sFileAbsURI): void {