From b425fbc2a28341a5627928f963519006712c3d39 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Mon, 11 Mar 2024 19:25:31 +0200 Subject: [PATCH] SECURITY: Generate more category CSS on client This commit moves the generation of category background CSS from the server side to the client side. This simplifies the server side code because it does not need to check which categories are visible to the current user. --- .../category-background-css-generator.js | 55 +++++++ .../category-background-css-generator-test.js | 132 ++++++++++++++++ lib/stylesheet/compiler.rb | 1 - lib/stylesheet/importer.rb | 30 ---- lib/stylesheet/manager.rb | 2 +- lib/stylesheet/manager/builder.rb | 11 +- spec/lib/stylesheet/importer_spec.rb | 145 ------------------ spec/lib/stylesheet/manager_spec.rb | 23 --- 8 files changed, 190 insertions(+), 209 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/instance-initializers/category-background-css-generator.js create mode 100644 app/assets/javascripts/discourse/tests/acceptance/category-background-css-generator-test.js diff --git a/app/assets/javascripts/discourse/app/instance-initializers/category-background-css-generator.js b/app/assets/javascripts/discourse/app/instance-initializers/category-background-css-generator.js new file mode 100644 index 0000000000000..5243bbbd4da2b --- /dev/null +++ b/app/assets/javascripts/discourse/app/instance-initializers/category-background-css-generator.js @@ -0,0 +1,55 @@ +import { getURLWithCDN } from "discourse-common/lib/get-url"; + +export default { + after: "register-hashtag-types", + + initialize(owner) { + this.session = owner.lookup("service:session"); + this.site = owner.lookup("service:site"); + + if (!this.site.categories?.length) { + return; + } + + const css = []; + const darkCss = []; + + this.site.categories.forEach((category) => { + const lightUrl = category.uploaded_background?.url; + const darkUrl = + this.session.defaultColorSchemeIsDark || this.session.darkModeAvailable + ? category.uploaded_background_dark?.url + : null; + const defaultUrl = + darkUrl && this.session.defaultColorSchemeIsDark ? darkUrl : lightUrl; + + if (defaultUrl) { + const url = getURLWithCDN(defaultUrl); + css.push( + `body.category-${category.fullSlug} { background-image: url(${url}); }` + ); + } + + if (darkUrl && defaultUrl !== darkUrl) { + const url = getURLWithCDN(darkUrl); + darkCss.push( + `body.category-${category.fullSlug} { background-image: url(${url}); }` + ); + } + }); + + if (darkCss.length > 0) { + css.push("@media (prefers-color-scheme: dark) {", ...darkCss, "}"); + } + + const cssTag = document.createElement("style"); + cssTag.type = "text/css"; + cssTag.id = "category-background-css-generator"; + cssTag.innerHTML = css.join("\n"); + document.head.appendChild(cssTag); + }, + + teardown() { + document.querySelector("#category-background-css-generator")?.remove(); + }, +}; diff --git a/app/assets/javascripts/discourse/tests/acceptance/category-background-css-generator-test.js b/app/assets/javascripts/discourse/tests/acceptance/category-background-css-generator-test.js new file mode 100644 index 0000000000000..7ba43e8578c67 --- /dev/null +++ b/app/assets/javascripts/discourse/tests/acceptance/category-background-css-generator-test.js @@ -0,0 +1,132 @@ +import { visit } from "@ember/test-helpers"; +import { test } from "qunit"; +import Session from "discourse/models/session"; +import { acceptance } from "discourse/tests/helpers/qunit-helpers"; + +const SITE_DATA = { + categories: [ + { + id: 1, + color: "ff0000", + text_color: "ffffff", + name: "category1", + slug: "foo", + uploaded_background: { + id: 54, + url: "/uploads/default/original/1X/c5c84b16ebf745ab848d1498267c541facbf1ff0.png", + width: 1024, + height: 768, + }, + }, + { + id: 2, + color: "333", + text_color: "ffffff", + name: "category2", + slug: "bar", + uploaded_background_dark: { + id: 25, + url: "/uploads/default/original/1X/f9fdb0ad108f2aed178c40f351bbb2c7cb2571e3.png", + width: 1024, + height: 768, + }, + }, + { + id: 4, + color: "2B81AF", + text_color: "ffffff", + parent_category_id: 1, + name: "category3", + slug: "baz", + uploaded_background: { + id: 11, + url: "/uploads/default/original/1X/684c104edc18a7e9cef1fa31f41215f3eec5d92b.png", + width: 1024, + height: 768, + }, + uploaded_background_dark: { + id: 19, + url: "/uploads/default/original/1X/89b1a2641e91604c32b21db496be11dba7a253e6.png", + width: 1024, + height: 768, + }, + }, + ], +}; + +acceptance("Category Background CSS Generator", function (needs) { + needs.user(); + needs.site(SITE_DATA); + + test("CSS classes are generated", async function (assert) { + await visit("/"); + + assert.equal( + document.querySelector("#category-background-css-generator").innerHTML, + "body.category-foo { background-image: url(/uploads/default/original/1X/c5c84b16ebf745ab848d1498267c541facbf1ff0.png); }\n" + + "body.category-foo-baz { background-image: url(/uploads/default/original/1X/684c104edc18a7e9cef1fa31f41215f3eec5d92b.png); }" + ); + }); +}); + +acceptance("Category Background CSS Generator (dark)", function (needs) { + needs.user(); + needs.site(SITE_DATA); + + needs.hooks.beforeEach(function () { + const session = Session.current(); + session.set("darkModeAvailable", true); + session.set("defaultColorSchemeIsDark", false); + }); + + needs.hooks.afterEach(function () { + const session = Session.current(); + session.set("darkModeAvailable", null); + session.set("defaultColorSchemeIsDark", null); + }); + + test("CSS classes are generated", async function (assert) { + await visit("/"); + + assert.equal( + document.querySelector("#category-background-css-generator").innerHTML, + "body.category-foo { background-image: url(/uploads/default/original/1X/c5c84b16ebf745ab848d1498267c541facbf1ff0.png); }\n" + + "body.category-foo-baz { background-image: url(/uploads/default/original/1X/684c104edc18a7e9cef1fa31f41215f3eec5d92b.png); }\n" + + "@media (prefers-color-scheme: dark) {\n" + + "body.category-bar { background-image: url(/uploads/default/original/1X/f9fdb0ad108f2aed178c40f351bbb2c7cb2571e3.png); }\n" + + "body.category-foo-baz { background-image: url(/uploads/default/original/1X/89b1a2641e91604c32b21db496be11dba7a253e6.png); }\n" + + "}" + ); + }); +}); + +acceptance( + "Category Background CSS Generator (dark is default)", + function (needs) { + needs.user(); + needs.site(SITE_DATA); + + needs.hooks.beforeEach(function () { + const session = Session.current(); + session.set("darkModeAvailable", true); + session.set("defaultColorSchemeIsDark", true); + }); + + needs.hooks.afterEach(function () { + const session = Session.current(); + session.set("darkModeAvailable", null); + session.set("defaultColorSchemeIsDark", null); + }); + + test("CSS classes are generated", async function (assert) { + await visit("/"); + + assert.equal( + document.querySelector("#category-background-css-generator").innerHTML, + "body.category-foo { background-image: url(/uploads/default/original/1X/c5c84b16ebf745ab848d1498267c541facbf1ff0.png); }\n" + + "body.category-bar { background-image: url(/uploads/default/original/1X/f9fdb0ad108f2aed178c40f351bbb2c7cb2571e3.png); }\n" + + "body.category-foo-baz { background-image: url(/uploads/default/original/1X/89b1a2641e91604c32b21db496be11dba7a253e6.png); }" + ); + }); + } +); diff --git a/lib/stylesheet/compiler.rb b/lib/stylesheet/compiler.rb index 1f839ae6ef8eb..583f69dbbd0aa 100644 --- a/lib/stylesheet/compiler.rb +++ b/lib/stylesheet/compiler.rb @@ -34,7 +34,6 @@ def self.compile_asset(asset, options = {}) when Stylesheet::Manager::COLOR_SCHEME_STYLESHEET file += importer.import_color_definitions file += importer.import_wcag_overrides - file += importer.category_backgrounds(options[:color_scheme_id]) file += importer.font end end diff --git a/lib/stylesheet/importer.rb b/lib/stylesheet/importer.rb index 90fdcdddf1e81..daab2cd31a4fb 100644 --- a/lib/stylesheet/importer.rb +++ b/lib/stylesheet/importer.rb @@ -95,20 +95,6 @@ def wizard_fonts contents end - def category_backgrounds(color_scheme_id) - is_dark_color_scheme = - color_scheme_id.present? && ColorScheme.find_by_id(color_scheme_id)&.is_dark? - - contents = +"" - Category - .where("uploaded_background_id IS NOT NULL") - .each do |c| - contents << category_css(c, is_dark_color_scheme) if c.uploaded_background&.url.present? - end - - contents - end - def import_color_definitions contents = +"" DiscoursePluginRegistry.color_definition_stylesheets.each do |name, path| @@ -220,22 +206,6 @@ def theme @theme == :nil ? nil : @theme end - def category_css(category, is_dark_color_scheme) - full_slug = category.full_slug.split("-")[0..-2].join("-") - - # in case we're using a dark color scheme, we define the background using the dark image - # if one is available. Otherwise, we use the light image by default. - if is_dark_color_scheme && category.uploaded_background_dark&.url.present? - return category_background_css(full_slug, category.uploaded_background_dark.url) - end - - category_background_css(full_slug, category.uploaded_background.url) - end - - def category_background_css(full_slug, background_url) - "body.category-#{full_slug} { background-image: url(#{upload_cdn_path(background_url)}) }" - end - def font_css(font) contents = +"" diff --git a/lib/stylesheet/manager.rb b/lib/stylesheet/manager.rb index a437330dade3d..04b1de3b696ec 100644 --- a/lib/stylesheet/manager.rb +++ b/lib/stylesheet/manager.rb @@ -7,7 +7,7 @@ module Stylesheet end class Stylesheet::Manager - BASE_COMPILER_VERSION = 1 + BASE_COMPILER_VERSION = 2 CACHE_PATH = "tmp/stylesheet-cache" private_constant :CACHE_PATH diff --git a/lib/stylesheet/manager/builder.rb b/lib/stylesheet/manager/builder.rb index 8bebf711bad13..738770de3cea6 100644 --- a/lib/stylesheet/manager/builder.rb +++ b/lib/stylesheet/manager/builder.rb @@ -240,20 +240,13 @@ def default_digest def color_scheme_digest cs = @color_scheme || theme&.color_scheme - categories_updated = - Stylesheet::Manager - .cache - .defer_get_set("categories_updated") do - Category.where("uploaded_background_id IS NOT NULL").pluck(:updated_at).map(&:to_i).sum - end - fonts = "#{SiteSetting.base_font}-#{SiteSetting.heading_font}" digest_string = "#{current_hostname}-" - if cs || categories_updated > 0 + if cs theme_color_defs = resolve_baked_field(:common, :color_definitions) digest_string += - "#{RailsMultisite::ConnectionManagement.current_db}-#{cs&.id}-#{cs&.version}-#{theme_color_defs}-#{Stylesheet::Manager.fs_asset_cachebuster}-#{categories_updated}-#{fonts}" + "#{RailsMultisite::ConnectionManagement.current_db}-#{cs&.id}-#{cs&.version}-#{theme_color_defs}-#{Stylesheet::Manager.fs_asset_cachebuster}-#{fonts}" else digest_string += "defaults-#{Stylesheet::Manager.fs_asset_cachebuster}-#{fonts}" diff --git a/spec/lib/stylesheet/importer_spec.rb b/spec/lib/stylesheet/importer_spec.rb index 53f1fc3837ce9..0f495a10c4d5b 100644 --- a/spec/lib/stylesheet/importer_spec.rb +++ b/spec/lib/stylesheet/importer_spec.rb @@ -7,151 +7,6 @@ def compile_css(name, options = {}) Stylesheet::Compiler.compile_asset(name, options)[0] end - describe "#category_backgrounds" do - it "uses the correct background image based in the color scheme" do - background = Fabricate(:upload) - background_dark = Fabricate(:upload) - - parent_category = Fabricate(:category) - category = - Fabricate( - :category, - parent_category_id: parent_category.id, - uploaded_background: background, - uploaded_background_dark: background_dark, - ) - - # light color schemes - ["Neutral", "Shades of Blue", "WCAG", "Summer", "Solarized Light"].each do |scheme_name| - scheme = ColorScheme.create_from_base(name: "Light Test", base_scheme_id: scheme_name) - - compiled_css = compile_css("color_definitions", { color_scheme_id: scheme.id }) - - expect(compiled_css).to include( - "body.category-#{parent_category.slug}-#{category.slug}{background-image:url(#{background.url})}", - ) - expect(compiled_css).not_to include(background_dark.url) - end - - # dark color schemes - [ - "Dark", - "Grey Amber", - "Latte", - "Dark Rose", - "WCAG Dark", - "Dracula", - "Solarized Dark", - ].each do |scheme_name| - scheme = ColorScheme.create_from_base(name: "Light Test", base_scheme_id: scheme_name) - - compiled_css = compile_css("color_definitions", { color_scheme_id: scheme.id }) - - expect(compiled_css).not_to include(background.url) - expect(compiled_css).to include( - "body.category-#{parent_category.slug}-#{category.slug}{background-image:url(#{background_dark.url})}", - ) - end - end - - it "applies CDN to background category images" do - expect(compile_css("color_definitions")).to_not include("body.category-") - - background = Fabricate(:upload) - background_dark = Fabricate(:upload) - - parent_category = Fabricate(:category) - category = - Fabricate( - :category, - parent_category_id: parent_category.id, - uploaded_background: background, - uploaded_background_dark: background_dark, - ) - - compiled_css = compile_css("color_definitions") - expect(compiled_css).to include( - "body.category-#{parent_category.slug}-#{category.slug}{background-image:url(#{background.url})}", - ) - - GlobalSetting.stubs(:cdn_url).returns("//awesome.cdn") - compiled_css = compile_css("color_definitions") - expect(compiled_css).to include( - "body.category-#{parent_category.slug}-#{category.slug}{background-image:url(//awesome.cdn#{background.url})}", - ) - end - - it "applies CDN to dark background category images" do - scheme = ColorScheme.create_from_base(name: "Dark Test", base_scheme_id: "Dark") - expect(compile_css("color_definitions", { color_scheme_id: scheme.id })).to_not include( - "body.category-", - ) - - background = Fabricate(:upload) - background_dark = Fabricate(:upload) - - parent_category = Fabricate(:category) - category = - Fabricate( - :category, - parent_category_id: parent_category.id, - uploaded_background: background, - uploaded_background_dark: background_dark, - ) - - compiled_css = compile_css("color_definitions", { color_scheme_id: scheme.id }) - expect(compiled_css).to include( - "body.category-#{parent_category.slug}-#{category.slug}{background-image:url(#{background_dark.url})}", - ) - - GlobalSetting.stubs(:cdn_url).returns("//awesome.cdn") - compiled_css = compile_css("color_definitions", { color_scheme_id: scheme.id }) - expect(compiled_css).to include( - "body.category-#{parent_category.slug}-#{category.slug}{background-image:url(//awesome.cdn#{background_dark.url})}", - ) - end - - it "applies S3 CDN to background category images" do - setup_s3 - SiteSetting.s3_use_iam_profile = true - SiteSetting.s3_upload_bucket = "test" - SiteSetting.s3_region = "ap-southeast-2" - SiteSetting.s3_cdn_url = "https://s3.cdn" - - background = Fabricate(:upload_s3) - category = Fabricate(:category, uploaded_background: background) - - compiled_css = compile_css("color_definitions") - expect(compiled_css).to include( - "body.category-#{category.slug}{background-image:url(https://s3.cdn/original", - ) - end - - it "applies S3 CDN to dark background category images" do - scheme = ColorScheme.create_from_base(name: "Dark Test", base_scheme_id: "WCAG Dark") - - setup_s3 - SiteSetting.s3_use_iam_profile = true - SiteSetting.s3_upload_bucket = "test" - SiteSetting.s3_region = "ap-southeast-2" - SiteSetting.s3_cdn_url = "https://s3.cdn" - - background = Fabricate(:upload_s3) - background_dark = Fabricate(:upload_s3) - category = - Fabricate( - :category, - uploaded_background: background, - uploaded_background_dark: background_dark, - ) - - compiled_css = compile_css("color_definitions", { color_scheme_id: scheme.id }) - expect(compiled_css).to include( - "body.category-#{category.slug}{background-image:url(https://s3.cdn/original", - ) - end - end - describe "#font" do it "includes font variable" do default_font = ":root{--font-family: Arial, sans-serif}" diff --git a/spec/lib/stylesheet/manager_spec.rb b/spec/lib/stylesheet/manager_spec.rb index d1be670fb172e..a53c53b29073e 100644 --- a/spec/lib/stylesheet/manager_spec.rb +++ b/spec/lib/stylesheet/manager_spec.rb @@ -483,29 +483,6 @@ def manager(theme_id = nil) describe "color_scheme_digest" do fab!(:theme) - it "changes with category background image" do - category1 = Fabricate(:category, uploaded_background_id: 123, updated_at: 1.week.ago) - category2 = Fabricate(:category, uploaded_background_id: 456, updated_at: 2.days.ago) - - manager = manager(theme.id) - - builder = - Stylesheet::Manager::Builder.new(target: :desktop_theme, theme: theme, manager: manager) - - digest1 = builder.color_scheme_digest - - category2.update!(uploaded_background_id: 789, updated_at: 1.day.ago) - - digest2 = builder.color_scheme_digest - expect(digest2).to_not eq(digest1) - - category1.update!(uploaded_background_id: nil, updated_at: 5.minutes.ago) - - digest3 = builder.color_scheme_digest - expect(digest3).to_not eq(digest2) - expect(digest3).to_not eq(digest1) - end - it "updates digest when updating a color scheme" do scheme = ColorScheme.create_from_base(name: "Neutral", base_scheme_id: "Neutral") manager = manager(theme.id)