diff --git a/app/controllers/admin/configs_controller.rb b/app/controllers/admin/configs_controller.rb index 3a08ca86c..59bf6b47f 100644 --- a/app/controllers/admin/configs_controller.rb +++ b/app/controllers/admin/configs_controller.rb @@ -125,10 +125,26 @@ module Admin home_feed_minimum_score ].freeze + IMAGE_FIELDS = + %w[ + main_social_image + logo_png + secondary_logo_url + campaign_sidebar_image + mascot_image_url + mascot_footer_image_url + onboarding_logo_image + onboarding_background_image + onboarding_taskcard_image + ].freeze + + VALID_URL = %r{\A(http|https)://([/|.|\w|\s|-])*.[a-z]{2,5}(:[0-9]{1,5})?(/.*)?\z}.freeze + layout "admin" before_action :extra_authorization_and_confirmation, only: [:create] before_action :validate_inputs, only: [:create] + before_action :validate_image_urls, only: [:create], if: -> { params[:site_config].keys & IMAGE_FIELDS } after_action :bust_content_change_caches, only: [:create] def show @@ -195,7 +211,15 @@ module Admin errors = [] errors << "Brand color must be darker for accessibility." if brand_contrast_too_low errors << "Brand color must be be a 6 character hex (starting with #)." if brand_color_not_hex - redirect_to admin_config_path, alert: "😭 #{errors.join(',')}" if errors.any? + redirect_to admin_config_path, alert: "😭 #{errors.to_sentence}" if errors.any? + end + + def validate_image_urls + image_params = config_params.slice(*IMAGE_FIELDS).to_h + errors = image_params.filter_map do |field, url| + "#{field} must be a valid URL" unless url.blank? || valid_image_url(url) + end + redirect_to admin_config_path, alert: "😭 #{errors.to_sentence}" if errors.any? end def clean_up_params @@ -235,5 +259,9 @@ module Admin hex = params.dig(:site_config, :primary_brand_color_hex) hex.present? && !hex.match?(/\A#(\h{6}|\h{3})\z/) end + + def valid_image_url(url) + url.match?(VALID_URL) + end end end diff --git a/spec/requests/admin/configs_spec.rb b/spec/requests/admin/configs_spec.rb index 56dcd31ea..5ac0a2bba 100644 --- a/spec/requests/admin/configs_spec.rb +++ b/spec/requests/admin/configs_spec.rb @@ -26,7 +26,7 @@ RSpec.describe "/admin/config", type: :request do end it "does not allow user to update config if they have proper confirmation" do - expected_image_url = "https://dummyimage.com/300x300" + expected_image_url = "https://dummyimage.com/300x300.png" expect do post "/admin/config", params: { site_config: { favicon_url: expected_image_url }, confirmation: confirmation_message } @@ -34,7 +34,7 @@ RSpec.describe "/admin/config", type: :request do end it "does not allow user to update config if they do not have proper confirmation" do - expected_image_url = "https://dummyimage.com/300x300" + expected_image_url = "https://dummyimage.com/300x300.png" expect do post "/admin/config", params: { site_config: { favicon_url: expected_image_url }, confirmation: "Not proper" } @@ -242,14 +242,29 @@ RSpec.describe "/admin/config", type: :request do expected_default_image_url = URL.local_image("social-media-cover.png") expect(SiteConfig.main_social_image).to eq(expected_default_image_url) - expected_image_url = "https://dummyimage.com/300x300" + expected_image_url = "https://dummyimage.com/300x300.png" post "/admin/config", params: { site_config: { main_social_image: expected_image_url }, confirmation: confirmation_message } expect(SiteConfig.main_social_image).to eq(expected_image_url) end + it "updates main_social_image with a valid image" do + expected_image = "https://dummyimage.com/300x300" + post "/admin/config", params: { site_config: { main_social_image: expected_image }, + confirmation: confirmation_message } + expect(SiteConfig.main_social_image).to eq(expected_image) + end + + it "only updates the main_social_image if given a valid image URL" do + invalid_image_url = "![logo_lowres]https://dummyimage.com/300x300" + expect do + post "/admin/config", params: { site_config: { main_social_image: invalid_image_url }, + confirmation: confirmation_message } + end.not_to change(SiteConfig, :main_social_image) + end + it "updates favicon_url" do - expected_image_url = "https://dummyimage.com/300x300" + expected_image_url = "https://dummyimage.com/300x300.png" post "/admin/config", params: { site_config: { favicon_url: expected_image_url }, confirmation: confirmation_message } expect(SiteConfig.favicon_url).to eq(expected_image_url) @@ -257,27 +272,57 @@ RSpec.describe "/admin/config", type: :request do it "updates logo_png" do expected_default_image_url = URL.local_image("icon.png") - expected_image_url = "https://dummyimage.com/300x300" + expected_image_url = "https://dummyimage.com/300x300.png" expect do post "/admin/config", params: { site_config: { logo_png: expected_image_url }, confirmation: confirmation_message } end.to change(SiteConfig, :logo_png).from(expected_default_image_url).to(expected_image_url) end + it "updates logo_png with a valid image" do + expected_image = "https://dummyimage.com/300x300" + post "/admin/config", params: { site_config: { logo_png: expected_image }, + confirmation: confirmation_message } + expect(SiteConfig.logo_png).to eq(expected_image) + end + + it "only updates the logo_png if given a valid image URL" do + invalid_image_url = "![logo_lowres]https://dummyimage.com/300x300.png" + expect do + post "/admin/config", params: { site_config: { logo_png: invalid_image_url }, + confirmation: confirmation_message } + end.not_to change(SiteConfig, :logo_png) + end + it "updates logo_svg" do - expected_image_url = "https://dummyimage.com/300x300" + expected_image_url = "https://dummyimage.com/300x300.png" post "/admin/config", params: { site_config: { logo_svg: expected_image_url }, confirmation: confirmation_message } expect(SiteConfig.logo_svg).to eq(expected_image_url) end it "updates secondary_logo_url" do - expected_image_url = "https://dummyimage.com/300x300" + expected_image_url = "https://dummyimage.com/300x300.png" post "/admin/config", params: { site_config: { secondary_logo_url: expected_image_url }, confirmation: confirmation_message } expect(SiteConfig.secondary_logo_url).to eq(expected_image_url) end + it "updates secondary_logo_url with a valid image" do + expected_image = "https://dummyimage.com/300x300" + post "/admin/config", params: { site_config: { secondary_logo_url: expected_image }, + confirmation: confirmation_message } + expect(SiteConfig.secondary_logo_url).to eq(expected_image) + end + + it "only updates the secondary_logo_url if given a valid image URL" do + invalid_image_url = "![logo_lowres]https://dummyimage.com/300x300.png" + expect do + post "/admin/config", params: { site_config: { secondary_logo_url: invalid_image_url }, + confirmation: confirmation_message } + end.not_to change(SiteConfig, :secondary_logo_url) + end + it "updates left_navbar_svg_icon" do expected_svg = "" @@ -295,7 +340,7 @@ RSpec.describe "/admin/config", type: :request do end it "rejects update without proper confirmation" do - expected_image_url = "https://dummyimage.com/300x300" + expected_image_url = "https://dummyimage.com/300x300.png" expect do post "/admin/config", params: { site_config: { logo_svg: expected_image_url }, confirmation: "Incorrect yo!" } @@ -303,7 +348,7 @@ RSpec.describe "/admin/config", type: :request do end it "rejects update without any confirmation" do - expected_image_url = "https://dummyimage.com/300x300" + expected_image_url = "https://dummyimage.com/300x300.png" expect do post "/admin/config", params: { site_config: { logo_svg: expected_image_url }, confirmation: "" } @@ -321,7 +366,7 @@ RSpec.describe "/admin/config", type: :request do it "updates mascot_image_url" do expected_default_image_url = URL.local_image("mascot.png") - expected_image_url = "https://dummyimage.com/300x300" + expected_image_url = "https://dummyimage.com/300x300.png" expect do post "/admin/config", params: { site_config: { mascot_image_url: expected_image_url }, confirmation: confirmation_message } @@ -329,7 +374,7 @@ RSpec.describe "/admin/config", type: :request do end it "updates mascot_footer_image_url" do - expected_image_url = "https://dummyimage.com/300x300" + expected_image_url = "https://dummyimage.com/300x300.png" post "/admin/config", params: { site_config: { mascot_footer_image_url: expected_image_url }, confirmation: confirmation_message } expect(SiteConfig.mascot_footer_image_url).to eq(expected_image_url) @@ -460,21 +505,21 @@ RSpec.describe "/admin/config", type: :request do describe "Onboarding" do it "updates onboarding_taskcard_image" do - expected_image_url = "https://dummyimage.com/300x300" + expected_image_url = "https://dummyimage.com/300x300.png" post "/admin/config", params: { site_config: { onboarding_taskcard_image: expected_image_url }, confirmation: confirmation_message } expect(SiteConfig.onboarding_taskcard_image).to eq(expected_image_url) end it "updates onboarding_logo_image" do - expected_image_url = "https://dummyimage.com/300x300" + expected_image_url = "https://dummyimage.com/300x300.png" post "/admin/config", params: { site_config: { onboarding_logo_image: expected_image_url }, confirmation: confirmation_message } expect(SiteConfig.onboarding_logo_image).to eq(expected_image_url) end it "updates onboarding_background_image" do - expected_image_url = "https://dummyimage.com/300x300" + expected_image_url = "https://dummyimage.com/300x300.png" post "/admin/config", params: { site_config: { onboarding_background_image: expected_image_url }, confirmation: confirmation_message } expect(SiteConfig.onboarding_background_image).to eq(expected_image_url)