diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index b6076636cfc..e24baddfff7 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -381,7 +381,8 @@ class ApplicationController < ActionController::Base theme_ids = [] if preview_theme_id = request[:preview_theme_id]&.to_i - theme_ids << preview_theme_id + ids = [preview_theme_id] + theme_ids = ids if guardian.allow_themes?(ids, include_preview: true) end user_option = current_user&.user_option @@ -394,10 +395,9 @@ class ApplicationController < ActionController::Base end end - theme_ids = user_option&.theme_ids || [] if theme_ids.blank? - - unless guardian.allow_themes?(theme_ids) - theme_ids = [] + if theme_ids.blank? + ids = user_option&.theme_ids || [] + theme_ids = ids if guardian.allow_themes?(ids) end if theme_ids.blank? && SiteSetting.default_theme_id != -1 diff --git a/lib/guardian.rb b/lib/guardian.rb index 8c7c6afb8d8..3f7a96eba36 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -364,10 +364,10 @@ class Guardian UserExport.where(user_id: @user.id, created_at: (Time.zone.now.beginning_of_day..Time.zone.now.end_of_day)).count == 0 end - def allow_themes?(theme_ids) + def allow_themes?(theme_ids, include_preview: false) return true if theme_ids.blank? - if is_staff? && (theme_ids - Theme.theme_ids).blank? + if include_preview && is_staff? && (theme_ids - Theme.theme_ids).blank? return true end diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb index 13ab35f426f..fc620ecc039 100644 --- a/spec/components/guardian_spec.rb +++ b/spec/components/guardian_spec.rb @@ -2581,8 +2581,11 @@ describe Guardian do let(:theme2) { Fabricate(:theme) } it "allows staff to use any themes" do - expect(Guardian.new(moderator).allow_themes?([theme.id, theme2.id])).to eq(true) - expect(Guardian.new(admin).allow_themes?([theme.id, theme2.id])).to eq(true) + 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) + + expect(Guardian.new(moderator).allow_themes?([theme.id, theme2.id], include_preview: true)).to eq(true) + expect(Guardian.new(admin).allow_themes?([theme.id, theme2.id], include_preview: true)).to eq(true) end it "only allows normal users to use user-selectable themes or default theme" do diff --git a/spec/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb index 03a998965dd..fb64f91b0b5 100644 --- a/spec/requests/application_controller_spec.rb +++ b/spec/requests/application_controller_spec.rb @@ -92,6 +92,7 @@ RSpec.describe ApplicationController do describe "#handle_theme" do let(:theme) { Fabricate(:theme, user_selectable: true) } let(:theme2) { Fabricate(:theme, user_selectable: true) } + let(:non_selectable_theme) { Fabricate(:theme, user_selectable: false) } let(:user) { Fabricate(:user) } let(:admin) { Fabricate(:admin) } @@ -148,6 +149,15 @@ RSpec.describe ApplicationController do get "/", params: { preview_theme_id: theme2.id } expect(response.status).to eq(200) expect(controller.theme_ids).to eq([theme2.id]) + + get "/", params: { preview_theme_id: non_selectable_theme.id } + expect(controller.theme_ids).to eq([non_selectable_theme.id]) + end + + it "does not allow non privileged user to preview themes" do + sign_in(user) + get "/", params: { preview_theme_id: non_selectable_theme.id } + expect(controller.theme_ids).to eq([SiteSetting.default_theme_id]) end it "cookie can fail back to user if out of sync" do