diff --git a/app/controllers/admin/themes_controller.rb b/app/controllers/admin/themes_controller.rb index d51b2439791..a08bc95495b 100644 --- a/app/controllers/admin/themes_controller.rb +++ b/app/controllers/admin/themes_controller.rb @@ -14,6 +14,9 @@ class Admin::ThemesController < Admin::AdminController end def upload_asset + + ban_in_whitelist_mode! + path = params[:file].path hijack do @@ -49,6 +52,9 @@ class Admin::ThemesController < Admin::AdminController def import @theme = nil if params[:theme] && params[:theme].content_type == "application/json" + + ban_in_whitelist_mode! + # .dcstyle.json import. Deprecated, but still available to allow conversion json = JSON::parse(params[:theme].read) theme = json['theme'] @@ -85,15 +91,21 @@ class Admin::ThemesController < Admin::AdminController else render json: @theme.errors, status: :unprocessable_entity end - elsif params[:remote] + elsif remote = params[:remote] + + guardian.ensure_allowed_theme_repo_import!(remote.strip) + begin branch = params[:branch] ? params[:branch] : nil - @theme = RemoteTheme.import_theme(params[:remote], theme_user, private_key: params[:private_key], branch: branch) + @theme = RemoteTheme.import_theme(remote, theme_user, private_key: params[:private_key], branch: branch) render json: @theme, status: :created rescue RemoteTheme::ImportError => e render_json_error e.message end elsif params[:bundle] || (params[:theme] && THEME_CONTENT_TYPES.include?(params[:theme].content_type)) + + ban_in_whitelist_mode! + # params[:bundle] used by theme CLI. params[:theme] used by admin UI bundle = params[:bundle] || params[:theme] theme_id = params[:theme_id] @@ -139,6 +151,9 @@ class Admin::ThemesController < Admin::AdminController end def create + + ban_in_whitelist_mode! + @theme = Theme.new(name: theme_params[:name], user_id: theme_user.id, user_selectable: theme_params[:user_selectable] || false, @@ -282,6 +297,10 @@ class Admin::ThemesController < Admin::AdminController private + def ban_in_whitelist_mode! + raise Discourse::InvalidAccess if !GlobalSetting.whitelisted_theme_ids.nil? + end + def add_relative_themes!(kind, ids) expected = ids.map(&:to_i) @@ -339,6 +358,8 @@ class Admin::ThemesController < Admin::AdminController def set_fields return unless fields = theme_params[:theme_fields] + ban_in_whitelist_mode! + fields.each do |field| @theme.set_field( target: field[:target], diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index e7f8c9750b0..2ce9f875ffd 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -414,7 +414,9 @@ class ApplicationController < ActionController::Base end if theme_ids.blank? && SiteSetting.default_theme_id != -1 - theme_ids << SiteSetting.default_theme_id + if guardian.allow_themes?([SiteSetting.default_theme_id]) + theme_ids << SiteSetting.default_theme_id + end end @theme_ids = request.env[:resolved_theme_ids] = theme_ids diff --git a/app/models/global_setting.rb b/app/models/global_setting.rb index cd80df66818..54dc310e618 100644 --- a/app/models/global_setting.rb +++ b/app/models/global_setting.rb @@ -220,6 +220,23 @@ class GlobalSetting end end + # test only + def self.reset_whitelisted_theme_ids! + @whitelisted_theme_ids = nil + end + + def self.whitelisted_theme_ids + return nil if whitelisted_theme_repos.blank? + + @whitelisted_theme_ids ||= begin + urls = whitelisted_theme_repos.split(",").map(&:strip) + Theme + .joins(:remote_theme) + .where('remote_themes.remote_url in (?)', urls) + .pluck(:id) + end + end + def self.add_default(name, default) unless self.respond_to? name define_singleton_method(name) do diff --git a/config/discourse_defaults.conf b/config/discourse_defaults.conf index 39df104204d..b2b2aff03ed 100644 --- a/config/discourse_defaults.conf +++ b/config/discourse_defaults.conf @@ -285,3 +285,12 @@ compress_anon_cache = false # This ensures there are no pathological cases where we keep storing data in anonymous cache # never to use it, set to 1 to store immediately, set to 0 to disable anon cache anon_cache_store_threshold = 2 + +# EXPERIMENTAL - not yet supported in production +# by default admins can install and amend any theme +# you may restrict it so only specific themes are approved +# in whitelist mode all theme updates must happen via git repos +# themes missing from the list are automatically disallowed +# list is a comma seperated list of git repos eg: +# https://github.com/discourse/discourse-custom-header-links.git,https://github.com/discourse/discourse-simple-theme.git +whitelisted_theme_repos = diff --git a/lib/guardian.rb b/lib/guardian.rb index 456461fead6..da6d1bf2b79 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -471,9 +471,27 @@ class Guardian @user.staff? || @user.trust_level >= TrustLevel.levels[:member] end + def allowed_theme_repo_import?(repo) + return false if !@user.admin? + + whitelisted_repos = GlobalSetting.whitelisted_theme_repos + if !whitelisted_repos.blank? + urls = whitelisted_repos.split(",").map(&:strip) + return urls.include?(repo) + end + + true + end + def allow_themes?(theme_ids, include_preview: false) return true if theme_ids.blank? + if whitelisted_theme_ids = GlobalSetting.whitelisted_theme_ids + if (theme_ids - whitelisted_theme_ids).present? + return false + end + end + if include_preview && is_staff? && (theme_ids - Theme.theme_ids).blank? return true end diff --git a/lib/tasks/themes.rake b/lib/tasks/themes.rake index 8a90196faf9..8a8cde8f68d 100644 --- a/lib/tasks/themes.rake +++ b/lib/tasks/themes.rake @@ -50,3 +50,27 @@ task "themes:install" => :environment do |task, args| exit 1 end end + +desc "List all the installed themes on the site" +task "themes:audit" => :environment do + components = Set.new + puts "Selectable themes" + puts "-----------------" + + Theme.where("(enabled OR user_selectable) AND NOT component").each do |theme| + puts theme.remote_theme&.remote_url || theme.name + theme.child_themes.each do |child| + if child.enabled + repo = child.remote_theme&.remote_url || child.name + components << repo + end + end + end + + puts + puts "Selectable components" + puts "---------------------" + components.each do |repo| + puts repo + end +end diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb index 97a1b0c2e06..53b1b14c1c3 100644 --- a/spec/components/guardian_spec.rb +++ b/spec/components/guardian_spec.rb @@ -2869,6 +2869,32 @@ describe Guardian do let!(:theme) { Fabricate(:theme) } let!(:theme2) { Fabricate(:theme) } + context "whitelist mode" do + before do + GlobalSetting.reset_whitelisted_theme_ids! + global_setting :whitelisted_theme_repos, " https://magic.com/repo.git, https://x.com/git" + end + + after do + GlobalSetting.reset_whitelisted_theme_ids! + end + + it "should respect theme whitelisting" do + r = RemoteTheme.create!(remote_url: "https://magic.com/repo.git") + theme.update!(remote_theme_id: r.id) + + guardian = Guardian.new(admin) + + expect(guardian.allow_themes?([theme.id, theme2.id], include_preview: true)).to eq(false) + + expect(guardian.allow_themes?([theme.id], include_preview: true)).to eq(true) + + expect(guardian.allowed_theme_repo_import?('https://x.com/git')).to eq(true) + expect(guardian.allowed_theme_repo_import?('https:/evil.com/git')).to eq(false) + + end + end + it "allows staff to use any themes" do expect(Guardian.new(moderator).allow_themes?([theme.id, theme2.id])).to eq(false) expect(Guardian.new(admin).allow_themes?([theme.id, theme2.id])).to eq(false) diff --git a/spec/requests/admin/themes_controller_spec.rb b/spec/requests/admin/themes_controller_spec.rb index 39e24e89401..a7d42ce4f9b 100644 --- a/spec/requests/admin/themes_controller_spec.rb +++ b/spec/requests/admin/themes_controller_spec.rb @@ -87,6 +87,7 @@ describe Admin::ThemesController do end describe '#import' do + let(:theme_json_file) do Rack::Test::UploadedFile.new(file_from_fixtures("sam-s-simple-theme.dcstyle.json", "json"), "application/json") end @@ -99,6 +100,40 @@ describe Admin::ThemesController do file_from_fixtures("logo.png") end + context 'when theme whitelist mode is enabled' do + before do + GlobalSetting.reset_whitelisted_theme_ids! + global_setting :whitelisted_theme_repos, "https://github.com/discourse/discourse-brand-header" + end + + after do + GlobalSetting.reset_whitelisted_theme_ids! + end + + it "allows whitelisted imports" do + RemoteTheme.stubs(:import_theme) + post "/admin/themes/import.json", params: { + remote: ' https://github.com/discourse/discourse-brand-header ' + } + + expect(response.status).to eq(201) + end + + it "bans non whtielisted imports" do + RemoteTheme.stubs(:import_theme) + post "/admin/themes/import.json", params: { + remote: ' https://bad.com/discourse/discourse-brand-header ' + } + + expect(response.status).to eq(403) + end + + it "bans json file import" do + post "/admin/themes/import.json", params: { theme: theme_json_file } + expect(response.status).to eq(403) + end + end + it 'can import a theme from Git' do RemoteTheme.stubs(:import_theme) post "/admin/themes/import.json", params: { @@ -120,7 +155,7 @@ describe Admin::ThemesController do end it 'imports a theme from an archive' do - existing_theme = Fabricate(:theme, name: "Header Icons") + _existing_theme = Fabricate(:theme, name: "Header Icons") expect do post "/admin/themes/import.json", params: { theme: theme_archive } @@ -135,7 +170,7 @@ describe Admin::ThemesController do it 'updates an existing theme from an archive by name' do # Old theme CLI method, remove Jan 2020 - existing_theme = Fabricate(:theme, name: "Header Icons") + _existing_theme = Fabricate(:theme, name: "Header Icons") expect do post "/admin/themes/import.json", params: { bundle: theme_archive } @@ -150,7 +185,7 @@ describe Admin::ThemesController do it 'updates an existing theme from an archive by id' do # Used by theme CLI - existing_theme = Fabricate(:theme, name: "Header Icons") + _existing_theme = Fabricate(:theme, name: "Header Icons") other_existing_theme = Fabricate(:theme, name: "Some other name") messages = MessageBus.track_publish do @@ -273,6 +308,34 @@ describe Admin::ThemesController do expect(SiteSetting.default_theme_id).to eq(-1) end + context 'when theme whitelist mode is enabled' do + before do + GlobalSetting.reset_whitelisted_theme_ids! + global_setting :whitelisted_theme_repos, " https://magic.com/repo.git, https://x.com/git" + end + + after do + GlobalSetting.reset_whitelisted_theme_ids! + end + + it 'unconditionally bans theme_fields from updating' do + r = RemoteTheme.create!(remote_url: "https://magic.com/repo.git") + theme.update!(remote_theme_id: r.id) + + put "/admin/themes/#{theme.id}.json", params: { + theme: { + name: 'my test name', + theme_fields: [ + { name: 'scss', target: 'common', value: '' }, + { name: 'scss', target: 'desktop', value: 'body{color: blue;}' }, + ] + } + } + + expect(response.status).to eq(403) + end + end + it 'updates a theme' do theme.set_field(target: :common, name: :scss, value: '.body{color: black;}') theme.save