From d1474e94a1bd46c9ddf21ba10c1270eb60cb3812 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Wed, 11 Mar 2020 13:30:45 +0000 Subject: [PATCH] FEATURE: Allow themes to specify modifiers in their about.json file (#9097) There are three modifiers: - serialize_topic_excerpts (boolean) - csp_extensions (array of strings) - svg_icons (array of strings) When multiple themes are active, the values will be combined. The combination method varies based on the setting. CSP/SVG arrays will be combined. serialize_topic_excerpts will use `Enumerable#any`. --- app/models/remote_theme.rb | 7 ++ app/models/theme.rb | 27 +++++++- app/models/theme_modifier_set.rb | 69 +++++++++++++++++++ app/serializers/listable_topic_serializer.rb | 2 +- config/locales/server.en.yml | 1 + .../20200302120829_add_theme_modifier_set.rb | 11 +++ lib/content_security_policy/extension.rb | 8 ++- lib/svg_sprite/svg_sprite.rb | 2 + lib/theme_modifier_helper.rb | 12 ++++ spec/components/svg_sprite/svg_sprite_spec.rb | 10 +++ .../theme_store/zip_exporter_spec.rb | 1 + spec/lib/content_security_policy_spec.rb | 15 ++++ spec/lib/theme_flag_modifier_spec.rb | 17 +++++ spec/models/remote_theme_spec.rb | 5 ++ spec/models/theme_modifier_set_spec.rb | 24 +++++++ 15 files changed, 205 insertions(+), 6 deletions(-) create mode 100644 app/models/theme_modifier_set.rb create mode 100644 db/migrate/20200302120829_add_theme_modifier_set.rb create mode 100644 lib/theme_modifier_helper.rb create mode 100644 spec/lib/theme_flag_modifier_spec.rb create mode 100644 spec/models/theme_modifier_set_spec.rb diff --git a/app/models/remote_theme.rb b/app/models/remote_theme.rb index 24336ee4ff0..6afa44c2a16 100644 --- a/app/models/remote_theme.rb +++ b/app/models/remote_theme.rb @@ -145,6 +145,13 @@ class RemoteTheme < ActiveRecord::Base raise ImportError, I18n.t("themes.import_error.about_json_values", errors: self.errors.full_messages.join(",")) end + ThemeModifierSet::MODIFIERS.keys.each do |modifier_name| + theme.theme_modifier_set.public_send(:"#{modifier_name}=", theme_info.dig("modifiers", modifier_name.to_s)) + end + if !theme.theme_modifier_set.valid? + raise ImportError, I18n.t("themes.import_error.modifier_values", errors: theme.theme_modifier_set.errors.full_messages.join(",")) + end + importer.all_files.each do |filename| next unless opts = ThemeField.opts_from_file_path(filename) value = importer[filename] diff --git a/app/models/theme.rb b/app/models/theme.rb index cce0005a607..35fd6630925 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -17,6 +17,7 @@ class Theme < ActiveRecord::Base has_many :parent_themes, -> { order(:name) }, through: :parent_theme_relation, source: :parent_theme has_many :color_schemes belongs_to :remote_theme, dependent: :destroy + has_one :theme_modifier_set, dependent: :destroy has_one :settings_field, -> { where(target_id: Theme.targets[:settings], name: "yaml") }, class_name: 'ThemeField' has_one :javascript_cache, dependent: :destroy @@ -34,6 +35,10 @@ class Theme < ActiveRecord::Base changed_schemes << scheme if scheme end + def theme_modifier_set + super || build_theme_modifier_set + end + after_save do changed_colors.each(&:save!) changed_schemes.each(&:save!) @@ -44,6 +49,8 @@ class Theme < ActiveRecord::Base changed_fields.each(&:save!) changed_fields.clear + theme_modifier_set.save! + if saved_change_to_name? theme_fields.select(&:basic_html_field?).each(&:invalidate_baked!) end @@ -112,8 +119,8 @@ class Theme < ActiveRecord::Base end def self.get_set_cache(key, &blk) - if val = @cache[key] - return val + if @cache.hash.key? key.to_s + return @cache[key] end @cache[key] = blk.call end @@ -247,6 +254,15 @@ class Theme < ActiveRecord::Base (@cache[cache_key] = val || "").html_safe end + def self.lookup_modifier(theme_ids, modifier_name) + theme_ids = [theme_ids] unless Array === theme_ids + + theme_ids = transform_ids(theme_ids) + get_set_cache("#{theme_ids.join(",")}:modifier:#{modifier_name}:#{ThemeField::COMPILER_VERSION}") do + ThemeModifierSet.resolve_modifier_for_themes(theme_ids, modifier_name) + end + end + def self.remove_from_cache! clear_cache! end @@ -521,6 +537,13 @@ class Theme < ActiveRecord::Base end end + meta[:modifiers] = {}.tap do |hash| + ThemeModifierSet::MODIFIERS.keys.each do |modifier| + value = self.theme_modifier_set.public_send(modifier) + hash[modifier] = value if !value.nil? + end + end + meta[:learn_more] = "https://meta.discourse.org/t/beginners-guide-to-using-discourse-themes/91966" end diff --git a/app/models/theme_modifier_set.rb b/app/models/theme_modifier_set.rb new file mode 100644 index 00000000000..262b0638ae0 --- /dev/null +++ b/app/models/theme_modifier_set.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true +class ThemeModifierSet < ActiveRecord::Base + class ThemeModifierSetError < StandardError; end + + belongs_to :theme + + MODIFIERS ||= { + serialize_topic_excerpts: { combine_mode: :any, type: :boolean }, + csp_extensions: { combine_mode: :flatten, type: :string_array }, + svg_icons: { combine_mode: :flatten, type: :string_array }, + } + + validate :type_validator + + def type_validator + MODIFIERS.each do |k, config| + value = public_send(k) + next if value.nil? + + case config[:type] + when :boolean + next if [true, false].include?(value) + when :string_array + next if value.is_a?(Array) && value.all? { |v| v.is_a?(String) } + end + errors.add(k, :invalid) + end + end + + after_save do + if saved_change_to_svg_icons? + SvgSprite.expire_cache + end + if saved_change_to_csp_extensions? + CSP::Extension.clear_theme_extensions_cache! + end + end + + # Given the ids of multiple active themes / theme components, this function + # will combine them into a 'resolved' behavior + def self.resolve_modifier_for_themes(theme_ids, modifier_name) + return nil if !(config = MODIFIERS[modifier_name]) + + all_values = self.where(theme_id: theme_ids).where.not(modifier_name => nil).pluck(modifier_name) + case config[:combine_mode] + when :any + all_values.any? + when :flatten + all_values.flatten(1) + else + raise ThemeModifierSetError "Invalid theme modifier combine_mode" + end + end +end + +# == Schema Information +# +# Table name: theme_modifier_sets +# +# id :bigint not null, primary key +# theme_id :bigint not null +# serialize_topic_excerpts :boolean +# csp_extensions :string is an Array +# svg_icons :string is an Array +# +# Indexes +# +# index_theme_modifier_sets_on_theme_id (theme_id) UNIQUE +# diff --git a/app/serializers/listable_topic_serializer.rb b/app/serializers/listable_topic_serializer.rb index 8a169eb1945..a00cd2f619d 100644 --- a/app/serializers/listable_topic_serializer.rb +++ b/app/serializers/listable_topic_serializer.rb @@ -111,7 +111,7 @@ class ListableTopicSerializer < BasicTopicSerializer alias :include_new_posts? :has_user_data def include_excerpt? - pinned || SiteSetting.always_include_topic_excerpts + pinned || SiteSetting.always_include_topic_excerpts || ThemeModifierHelper.new(request: scope.request).serialize_topic_excerpts end def pinned diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index f4f234f8ecb..0b1011a46cd 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -83,6 +83,7 @@ en: generic: An error occured while importing that theme about_json: "Import Error: about.json does not exist, or is invalid" about_json_values: "about.json contains invalid values: %{errors}" + flag_values: "about.json flags contain invalid values: %{errors}" git: "Error cloning git repository, access is denied or repository is not found" unpack_failed: "Failed to unpack file" file_too_big: "The uncompressed file is too big." diff --git a/db/migrate/20200302120829_add_theme_modifier_set.rb b/db/migrate/20200302120829_add_theme_modifier_set.rb new file mode 100644 index 00000000000..a01be519856 --- /dev/null +++ b/db/migrate/20200302120829_add_theme_modifier_set.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true +class AddThemeModifierSet < ActiveRecord::Migration[6.0] + def change + create_table(:theme_modifier_sets) do |t| + t.references :theme, index: { unique: true }, null: false + t.column :serialize_topic_excerpts, :boolean + t.column :csp_extensions, :string, array: true + t.column :svg_icons, :string, array: true + end + end +end diff --git a/lib/content_security_policy/extension.rb b/lib/content_security_policy/extension.rb index f782f91e26f..5dd70223d06 100644 --- a/lib/content_security_policy/extension.rb +++ b/lib/content_security_policy/extension.rb @@ -44,16 +44,18 @@ class ContentSecurityPolicy Theme.where(id: Theme.transform_ids(theme_ids)).find_each do |theme| theme.cached_settings.each do |setting, value| - extensions << build_theme_extension(value) if setting.to_s == THEME_SETTING + extensions << build_theme_extension(value.split("|")) if setting.to_s == THEME_SETTING end end + extensions << build_theme_extension(ThemeModifierHelper.new(theme_ids: theme_ids).csp_extensions) + extensions end - def build_theme_extension(raw) + def build_theme_extension(entries) {}.tap do |extension| - raw.split('|').each do |entry| + entries.each do |entry| directive, source = entry.split(':', 2).map(&:strip) extension[directive] ||= [] diff --git a/lib/svg_sprite/svg_sprite.rb b/lib/svg_sprite/svg_sprite.rb index 264154d3a63..8751f114dcb 100644 --- a/lib/svg_sprite/svg_sprite.rb +++ b/lib/svg_sprite/svg_sprite.rb @@ -399,6 +399,8 @@ License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL end end + theme_icon_settings |= ThemeModifierHelper.new(theme_ids: theme_ids).svg_icons + theme_icon_settings end diff --git a/lib/theme_modifier_helper.rb b/lib/theme_modifier_helper.rb new file mode 100644 index 00000000000..3d72a6d35aa --- /dev/null +++ b/lib/theme_modifier_helper.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true +class ThemeModifierHelper + def initialize(request: nil, theme_ids: nil) + @theme_ids = theme_ids || request&.env&.[](:resolved_theme_ids) + end + + ThemeModifierSet::MODIFIERS.keys.each do |modifier| + define_method(modifier) do + Theme.lookup_modifier(@theme_ids, modifier) + end + end +end diff --git a/spec/components/svg_sprite/svg_sprite_spec.rb b/spec/components/svg_sprite/svg_sprite_spec.rb index f8acc0028b8..a9614c55481 100644 --- a/spec/components/svg_sprite/svg_sprite_spec.rb +++ b/spec/components/svg_sprite/svg_sprite_spec.rb @@ -142,6 +142,16 @@ describe SvgSprite do expect(SvgSprite.all_icons([parent_theme.id])).to include("dragon") end + it 'includes icons defined in theme flags' do + theme = Fabricate(:theme) + + expect(SvgSprite.all_icons([theme.id])).not_to include("dragon") + + theme.theme_modifier_set.svg_icons = ["dragon"] + theme.save! + expect(SvgSprite.all_icons([theme.id])).to include("dragon") + end + it 'includes custom icons from a sprite in a theme' do theme = Fabricate(:theme) fname = "custom-theme-icon-sprite.svg" diff --git a/spec/components/theme_store/zip_exporter_spec.rb b/spec/components/theme_store/zip_exporter_spec.rb index e8c460709b9..30f41806c93 100644 --- a/spec/components/theme_store/zip_exporter_spec.rb +++ b/spec/components/theme_store/zip_exporter_spec.rb @@ -100,6 +100,7 @@ describe ThemeStore::ZipExporter do "tertiary": "858585" } }, + "modifiers": {}, "learn_more": "https://meta.discourse.org/t/beginners-guide-to-using-discourse-themes/91966" ) diff --git a/spec/lib/content_security_policy_spec.rb b/spec/lib/content_security_policy_spec.rb index a40865a4814..fa81698019c 100644 --- a/spec/lib/content_security_policy_spec.rb +++ b/spec/lib/content_security_policy_spec.rb @@ -165,6 +165,21 @@ describe ContentSecurityPolicy do expect(parse(theme_policy)['script-src']).to_not include('https://from-theme.net') expect(parse(theme_policy)['worker-src']).to_not include('from-theme.com') end + + it 'can be extended by theme flags' do + policy # call this first to make sure further actions clear the cache + + theme.theme_modifier_set.csp_extensions = ["script-src: https://from-theme-flag.script", "worker-src: from-theme-flag.worker"] + theme.save! + + expect(parse(theme_policy)['script-src']).to include('https://from-theme-flag.script') + expect(parse(theme_policy)['worker-src']).to include('from-theme-flag.worker') + + theme.destroy! + + expect(parse(theme_policy)['script-src']).to_not include('https://from-theme-flag.script') + expect(parse(theme_policy)['worker-src']).to_not include('from-theme-flag.worker') + end end it 'can be extended by site setting' do diff --git a/spec/lib/theme_flag_modifier_spec.rb b/spec/lib/theme_flag_modifier_spec.rb new file mode 100644 index 00000000000..574c20db860 --- /dev/null +++ b/spec/lib/theme_flag_modifier_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true +require 'rails_helper' + +describe ThemeModifierHelper do + fab!(:theme) { Fabricate(:theme).tap { |t| t.theme_modifier_set.update!(serialize_topic_excerpts: true) } } + + it "defines a getter for modifiers" do + tmh = ThemeModifierHelper.new(theme_ids: [theme.id]) + expect(tmh.serialize_topic_excerpts).to eq(true) + end + + it "can extract theme ids from a request object" do + request = Rack::Request.new({ resolved_theme_ids: [theme.id] }) + tmh = ThemeModifierHelper.new(request: request) + expect(tmh.serialize_topic_excerpts).to eq(true) + end +end diff --git a/spec/models/remote_theme_spec.rb b/spec/models/remote_theme_spec.rb index 6f5bab1df19..29cc4b939d5 100644 --- a/spec/models/remote_theme_spec.rb +++ b/spec/models/remote_theme_spec.rb @@ -37,6 +37,9 @@ describe RemoteTheme do "love": "#{love_color}", "tertiary-low": "#{tertiary_low_color}" } + }, + "modifiers": { + "serialize_topic_excerpts": true } } JSON @@ -85,6 +88,8 @@ describe RemoteTheme do expect(remote.theme_version).to eq("1.0") expect(remote.minimum_discourse_version).to eq("1.0.0") + expect(@theme.theme_modifier_set.serialize_topic_excerpts).to eq(true) + expect(@theme.theme_fields.length).to eq(9) mapped = Hash[*@theme.theme_fields.map { |f| ["#{f.target_id}-#{f.name}", f.value] }.flatten] diff --git a/spec/models/theme_modifier_set_spec.rb b/spec/models/theme_modifier_set_spec.rb new file mode 100644 index 00000000000..ca770f339fb --- /dev/null +++ b/spec/models/theme_modifier_set_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true +require 'rails_helper' + +describe ThemeModifierSet do + describe "#resolve_modifiers_for_themes" do + it "returns nil for unknown modifier" do + expect(ThemeModifierSet.resolve_modifier_for_themes([1, 2], :unknown_modifier)).to eq(nil) + end + + it "resolves serialize_topic_excerpts correctly" do + t1 = Fabricate(:theme) + t1.theme_modifier_set.update!(serialize_topic_excerpts: true) + t2 = Fabricate(:theme) + t2.theme_modifier_set.update!(serialize_topic_excerpts: false) + + expect(ThemeModifierSet.resolve_modifier_for_themes([t1.id, t2.id], :serialize_topic_excerpts)).to eq(true) + + t1 = Fabricate(:theme) + t1.theme_modifier_set.update!(serialize_topic_excerpts: nil) + + expect(ThemeModifierSet.resolve_modifier_for_themes([t1.id, t2.id], :serialize_topic_excerpts)).to eq(false) + end + end +end