Merge pull request #7124 from weseek/feat/109763-save-prevent-xss-custom-settings-in-new-format

feat: Save prevent xss custom settings in new format

packages/app/src/client/services/AdminMarkDownContainer.js

@@ -29,7 +29,7 @@ export default class AdminMarkDownContainer extends Container {
       isEnabledXss: false,
       xssOption: '',
       tagWhiteList: '',
-      attrWhiteList: '',
+      attrWhiteList: '{}',
     };
 
     this.switchEnableXss = this.switchEnableXss.bind(this);
@@ -119,19 +119,25 @@ export default class AdminMarkDownContainer extends Container {
    * Update Xss Setting
    */
   async updateXssSetting() {
-    let { tagWhiteList, attrWhiteList } = this.state;
+    let { tagWhiteList } = this.state;
+    const { attrWhiteList } = this.state;
 
     tagWhiteList = Array.isArray(tagWhiteList) ? tagWhiteList : tagWhiteList.split(',');
-    attrWhiteList = Array.isArray(attrWhiteList) ? attrWhiteList : attrWhiteList.split(',');
 
-    const response = await apiv3Put('/markdown-setting/xss', {
+    try {
+      // Check if parsing is possible
+      JSON.parse(attrWhiteList);
+    }
+    catch (err) {
+      throw Error(err);
+    }
+
+    await apiv3Put('/markdown-setting/xss', {
       isEnabledXss: this.state.isEnabledXss,
       xssOption: this.state.xssOption,
       tagWhiteList,
-      attrWhiteList,
+      attrWhiteList: attrWhiteList ?? '{}',
     });
-
-    return response;
   }
 
   /**

packages/app/src/server/models/config.ts

@@ -147,12 +147,14 @@ export const defaultCrowiConfigs: { [key: string]: any } = {
 };
 
 export const defaultMarkdownConfigs: { [key: string]: any } = {
+  // don't use it, but won't turn it off
   'markdown:xss:tagWhiteList': [],
   'markdown:xss:attrWhiteList': [],
+
   'markdown:rehypeSanitize:isEnabledPrevention': true,
   'markdown:rehypeSanitize:option': RehypeSanitizeOption.RECOMMENDED,
   'markdown:rehypeSanitize:tagNames': [],
-  'markdown:rehypeSanitize:attributes': {},
+  'markdown:rehypeSanitize:attributes': '{}',
   'markdown:isEnabledLinebreaks': false,
   'markdown:isEnabledLinebreaksInComments': true,
   'markdown:adminPreferredIndentSize': 4,

packages/app/src/server/routes/apiv3/markdown-setting.js

@@ -30,7 +30,7 @@ const validator = {
   xssSetting: [
     body('isEnabledXss').isBoolean(),
     body('tagWhiteList').isArray(),
-    body('attrWhiteList').isArray(),
+    body('attrWhiteList').isString(),
   ],
 };
 
@@ -127,8 +127,8 @@ module.exports = (crowi) => {
       pageBreakCustomSeparator: await crowi.configManager.getConfig('markdown', 'markdown:presentation:pageBreakCustomSeparator'),
       isEnabledXss: await crowi.configManager.getConfig('markdown', 'markdown:rehypeSanitize:isEnabledPrevention'),
       xssOption: await crowi.configManager.getConfig('markdown', 'markdown:rehypeSanitize:option'),
-      tagWhiteList: await crowi.configManager.getConfig('markdown', 'markdown:xss:tagWhiteList'),
-      attrWhiteList: await crowi.configManager.getConfig('markdown', 'markdown:xss:attrWhiteList'),
+      tagWhiteList: await crowi.configManager.getConfig('markdown', 'markdown:rehypeSanitize:tagNames'),
+      attrWhiteList: await crowi.configManager.getConfig('markdown', 'markdown:rehypeSanitize:attributes'),
     };
 
     return res.apiv3({ markdownParams });
@@ -292,11 +292,20 @@ module.exports = (crowi) => {
       return res.apiv3Err(new ErrorV3('xss option is required'));
     }
 
+    try {
+      JSON.parse(req.body.attrWhiteList);
+    }
+    catch (err) {
+      const msg = 'Error occurred in updating xss';
+      logger.error('Error', err);
+      return res.apiv3Err(new ErrorV3(msg, 'update-xss-failed'));
+    }
+
     const reqestXssParams = {
       'markdown:rehypeSanitize:isEnabledPrevention': req.body.isEnabledXss,
       'markdown:rehypeSanitize:option': req.body.xssOption,
-      'markdown:xss:tagWhiteList': req.body.tagWhiteList, // Todo: need to be changed at https://redmine.weseek.co.jp/issues/109763
-      'markdown:xss:attrWhiteList': req.body.attrWhiteList, // Todo: need to be changed at https://redmine.weseek.co.jp/issues/109763
+      'markdown:rehypeSanitize:tagNames': req.body.tagWhiteList,
+      'markdown:rehypeSanitize:attributes': req.body.attrWhiteList,
     };
 
     try {
@@ -304,8 +313,8 @@ module.exports = (crowi) => {
       const xssParams = {
         isEnabledXss: await crowi.configManager.getConfig('markdown', 'markdown:rehypeSanitize:isEnabledPrevention'),
         xssOption: await crowi.configManager.getConfig('markdown', 'markdown:rehypeSanitize:option'),
-        tagWhiteList: await crowi.configManager.getConfig('markdown', 'markdown:xss:tagWhiteList'),
-        attrWhiteList: await crowi.configManager.getConfig('markdown', 'markdown:xss:attrWhiteList'),
+        tagWhiteList: await crowi.configManager.getConfig('markdown', 'markdown:rehypeSanitize:tagNames'),
+        attrWhiteList: await crowi.configManager.getConfig('markdown', 'markdown:rehypeSanitize:attributes'),
       };
 
       const parameters = { action: SupportedAction.ACTION_ADMIN_MARKDOWN_XSS_UPDATE };