diff --git a/app/assets/javascripts/admin/addon/templates/components/site-settings/html.hbs b/app/assets/javascripts/admin/addon/templates/components/site-settings/html.hbs deleted file mode 100644 index 0da61a84e1f..00000000000 --- a/app/assets/javascripts/admin/addon/templates/components/site-settings/html.hbs +++ /dev/null @@ -1,2 +0,0 @@ -{{text-field value=(html-safe value) classNames="input-setting-string"}} -
{{html-safe setting.description}}
diff --git a/app/assets/javascripts/discourse/app/components/global-notice.js b/app/assets/javascripts/discourse/app/components/global-notice.js index 9dc041b8636..82936c7a59d 100644 --- a/app/assets/javascripts/discourse/app/components/global-notice.js +++ b/app/assets/javascripts/discourse/app/components/global-notice.js @@ -5,6 +5,7 @@ import I18n from "I18n"; import LogsNotice from "discourse/services/logs-notice"; import { bind } from "discourse-common/utils/decorators"; import getURL from "discourse-common/lib/get-url"; +import { htmlSafe } from "@ember/template"; const _pluginNotices = []; @@ -111,7 +112,9 @@ export default Component.extend({ const requiredText = I18n.t("wizard_required", { url: getURL("/wizard"), }); - notices.push(Notice.create({ text: requiredText, id: "alert-wizard" })); + notices.push( + Notice.create({ text: htmlSafe(requiredText), id: "alert-wizard" }) + ); } if ( @@ -214,7 +217,7 @@ export default Component.extend({ @bind _handleLogsNoticeUpdate() { const logNotice = Notice.create({ - text: LogsNotice.currentProp("message"), + text: htmlSafe(LogsNotice.currentProp("message")), id: "alert-logs-notice", options: { dismissable: true, diff --git a/app/assets/javascripts/discourse/app/templates/components/global-notice.hbs b/app/assets/javascripts/discourse/app/templates/components/global-notice.hbs index 08ff36603fc..96aee8f1771 100644 --- a/app/assets/javascripts/discourse/app/templates/components/global-notice.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/global-notice.hbs @@ -4,7 +4,7 @@ {{#if notice.options.html}} {{html-safe notice.options.html}} {{/if}} - {{html-safe notice.text}} + {{notice.text}} {{#if notice.options.dismissable}} {{d-button diff --git a/app/controllers/admin/site_settings_controller.rb b/app/controllers/admin/site_settings_controller.rb index b909adf06a7..a17f9c70de6 100644 --- a/app/controllers/admin/site_settings_controller.rb +++ b/app/controllers/admin/site_settings_controller.rb @@ -7,7 +7,7 @@ class Admin::SiteSettingsController < Admin::AdminController def index render_json_dump( - site_settings: SiteSetting.all_settings(sanitize_plain_text_settings: true), + site_settings: SiteSetting.all_settings, diags: SiteSetting.diags ) end diff --git a/config/site_settings.yml b/config/site_settings.yml index 2c98aad72f9..42f3c7ab07d 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -19,7 +19,6 @@ # type: username - Must match the username of an existing user. # type: list - A list of values, chosen from a set of valid values defined in the choices option. # type: enum - A single value, chosen from a set of valid values in the choices option. -# type: html - A single value. It could contain HTML. # # A type:list setting with the word 'colors' in its name will make color values have a bold line of the corresponding color # diff --git a/db/migrate/20210928161912_migrate_deprecated_html_setting_type.rb b/db/migrate/20210928161912_migrate_deprecated_html_setting_type.rb new file mode 100644 index 00000000000..de64efd108d --- /dev/null +++ b/db/migrate/20210928161912_migrate_deprecated_html_setting_type.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class MigrateDeprecatedHtmlSettingType < ActiveRecord::Migration[6.1] + def up + execute <<~SQL + UPDATE site_settings + SET data_type = 1 + WHERE data_type = 25 + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb index 28c7a9cd45c..dd529e8e71c 100644 --- a/lib/site_setting_extension.rb +++ b/lib/site_setting_extension.rb @@ -212,16 +212,12 @@ module SiteSettingExtension value = value.to_s if type == :upload value = value.map(&:to_s).join("|") if type == :uploaded_image_list - if should_sanitize?(value, type) - value = sanitize(value) - end - [name, value] end.flatten]) end # Retrieve all settings - def all_settings(include_hidden: false, sanitize_plain_text_settings: false) + def all_settings(include_hidden: false) locale_setting_hash = { @@ -250,8 +246,6 @@ module SiteSettingExtension default.to_i < Upload::SEEDED_ID_THRESHOLD default = default_uploads[default.to_i] - elsif sanitize_plain_text_settings && should_sanitize?(value, type_hash[:type].to_s) - value = sanitize(value) end opts = { @@ -582,14 +576,6 @@ module SiteSettingExtension end end - def should_sanitize?(value, type) - value.is_a?(String) && type.to_s != 'html' - end - - def sanitize(value) - CGI.unescapeHTML(Loofah.scrub_fragment(value, :strip).to_s) - end - def logger Rails.logger end diff --git a/lib/site_settings/type_supervisor.rb b/lib/site_settings/type_supervisor.rb index 88d07a34740..99170c3dc0d 100644 --- a/lib/site_settings/type_supervisor.rb +++ b/lib/site_settings/type_supervisor.rb @@ -37,7 +37,7 @@ class SiteSettings::TypeSupervisor color: 22, simple_list: 23, emoji_list: 24, - html: 25 + html_deprecated: 25, ) end diff --git a/spec/components/site_setting_extension_spec.rb b/spec/components/site_setting_extension_spec.rb index c34588725f1..5be49b0ff03 100644 --- a/spec/components/site_setting_extension_spec.rb +++ b/spec/components/site_setting_extension_spec.rb @@ -831,23 +831,6 @@ describe SiteSettingExtension do expect(setting[:default]).to eq(system_upload.url) end end - - it 'should sanitize html in the site settings' do - settings.setting(:with_html, 'rest') - settings.refresh! - - setting = settings.all_settings(sanitize_plain_text_settings: true).last - - expect(setting[:value]).to eq('rest') - end - - it 'settings with html type are not sanitized' do - settings.setting(:with_html, 'rest', type: :html) - - setting = settings.all_settings(sanitize_plain_text_settings: true).last - - expect(setting[:value]).to eq('rest') - end end describe '.client_settings_json_uncached' do @@ -862,19 +845,6 @@ describe SiteSettingExtension do ) end - it 'should sanitize html in the site settings' do - settings.setting(:with_html, 'rest', client: true) - settings.setting(:with_symbols, '<>rest', client: true) - settings.setting(:with_unknown_tag, 'rest', client: true) - settings.refresh! - - client_settings = JSON.parse settings.client_settings_json_uncached - - expect(client_settings['with_html']).to eq('rest') - expect(client_settings['with_symbols']).to eq('<>rest') - expect(client_settings['with_unknown_tag']).to eq('rest') - end - it 'settings with html type are not sanitized' do settings.setting(:with_html, 'rest', type: :html, client: true) diff --git a/spec/components/site_settings/type_supervisor_spec.rb b/spec/components/site_settings/type_supervisor_spec.rb index f565b86eb05..36c6f82a197 100644 --- a/spec/components/site_settings/type_supervisor_spec.rb +++ b/spec/components/site_settings/type_supervisor_spec.rb @@ -94,9 +94,6 @@ describe SiteSettings::TypeSupervisor do it "'emoji_list' should be at the right position" do expect(SiteSettings::TypeSupervisor.types[:emoji_list]).to eq(24) end - it "'html' should be at the right position" do - expect(SiteSettings::TypeSupervisor.types[:html]).to eq(25) - end end end