Validate Image URLs in SiteConfig (#11299) [deploy]
* Adds image URL validation to image URLs in configs_controller.rb * Adds additional image URLs to #valid_image_url in configs_controller.rb * Adds tests around valid image URLs to configs_spec.rb * WIP: sets strict conditions for before_action * WIP: comments out unused code for draft PR * WIP: cleans up configs_controller.rb for draft PR review * Renames constant to more explicit IMAGE_FIELDS in configs_controller.rb * Removes unnecessary comments from admin/configs_spec.rb * Checks for matching keys in params and IMAGE_FIELDS in configs_controller * Refactors #valid_image_urls and #valid_image_url in ConfigsController (THANK U, ANDY) * Refactors image-related code and error messages in Admin::ConfigsController * Renames expected_image_url to invalid_image_url and removes useless test * Fix: update the tests to contain valid image urls so that they pass the validation. * Adjusts tests & VALID_URL regex to validate images rather than image-specific URL * Replaces $ with \z per Rhymes suggestion and feedback * Adds accidentally removed code Co-authored-by: Ridhwana <ridhwana.khan16@gmail.com>
This commit is contained in:
Julianna Tetreault
GitHub
parent
60b7572f5c
commit
0ee74d1e26
2 changed files with 88 additions and 15 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 = "<svg height='100' width='100'><circle cx='50' cy='50' r='40' " \
|
||||
"stroke='black' stroke-width='3'/></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)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue