From 5496ac5b830841a1d724184b38e1ef4801c4002f Mon Sep 17 00:00:00 2001 From: Jacob Herrington Date: Wed, 26 Aug 2020 18:59:41 -0500 Subject: [PATCH] Make confirmation text a required field (#10014) * Make confirmation text a required field * Use strong params to check if confirmation param is present * Require config confirmation text to match pattern --- app/controllers/admin/configs_controller.rb | 15 +++++++++++++-- app/views/admin/configs/show.html.erb | 8 ++++---- spec/requests/admin/configs_spec.rb | 18 +++++++++++++----- 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/app/controllers/admin/configs_controller.rb b/app/controllers/admin/configs_controller.rb index 211d0ecac..4f5ca66f2 100644 --- a/app/controllers/admin/configs_controller.rb +++ b/app/controllers/admin/configs_controller.rb @@ -4,6 +4,10 @@ module Admin before_action :extra_authorization_and_confirmation, only: [:create] + def show + @confirmation_text = confirmation_text + end + def create clean_up_params @@ -23,6 +27,10 @@ module Admin private + def confirmation_text + "My username is @#{current_user.username} and this action is 100% safe and appropriate." + end + def config_params allowed_params = %i[ ga_view_id @@ -69,10 +77,13 @@ module Admin ) end + def raise_confirmation_mismatch_error + raise ActionController::BadRequest.new, "The confirmation key does not match" + end + def extra_authorization_and_confirmation not_authorized unless current_user.has_role?(:single_resource_admin, Config) # Special additional permission - confirmation_message = "My username is @#{current_user.username} and this action is 100% safe and appropriate." - not_authorized if params[:confirmation] != confirmation_message + raise_confirmation_mismatch_error if params.require(:confirmation) != confirmation_text end def clean_up_params diff --git a/app/views/admin/configs/show.html.erb b/app/views/admin/configs/show.html.erb index e328517fe..157798381 100644 --- a/app/views/admin/configs/show.html.erb +++ b/app/views/admin/configs/show.html.erb @@ -66,8 +66,8 @@ <% if current_user.has_role?(:single_resource_admin, Config) %>
<%= label_tag "Please type out the sentence below to submit:" %> - <%= text_field_tag :confirmation, nil, class: "form-control", placeholder: "Confirmation text", autocomplete: "off" %> -
Type the sentence: My username is @your_username and this action is 100% safe and appropriate.
+ <%= text_field_tag :confirmation, nil, class: "form-control", placeholder: "Confirmation text", autocomplete: "off", required: true, pattern: @confirmation_text %> +
Type the sentence: <%= @confirmation_text %>
<%= f.submit "Update Site Configuration", class: "btn btn-danger btn-lg" %>
<% end %> @@ -965,8 +965,8 @@
<%= label_tag :confirmation %> - <%= text_field_tag :confirmation, nil, class: "form-control", placeholder: "Confirmation text", autocomplete: "off" %> -
Type the sentence: My username is @your_username and this action is 100% safe and appropriate.
+ <%= text_field_tag :confirmation, nil, class: "form-control", placeholder: "Confirmation text", autocomplete: "off", required: true, pattern: @confirmation_text %> +
Type the sentence: <%= @confirmation_text %>
<%= f.submit "Update Site Configuration", class: "btn btn-danger btn-lg" %>
diff --git a/spec/requests/admin/configs_spec.rb b/spec/requests/admin/configs_spec.rb index fef75d2d2..46590355d 100644 --- a/spec/requests/admin/configs_spec.rb +++ b/spec/requests/admin/configs_spec.rb @@ -155,7 +155,7 @@ RSpec.describe "/admin/config", type: :request do expect do post "/admin/config", params: { site_config: { periodic_email_digest_min: 6 }, confirmation: "Incorrect yo!" } - end.to raise_error Pundit::NotAuthorizedError + end.to raise_error ActionController::BadRequest expect(SiteConfig.periodic_email_digest_min).not_to eq(6) end end @@ -238,7 +238,15 @@ RSpec.describe "/admin/config", type: :request do expect do post "/admin/config", params: { site_config: { logo_svg: expected_image_url }, confirmation: "Incorrect yo!" } - end.to raise_error Pundit::NotAuthorizedError + end.to raise_error ActionController::BadRequest + end + + it "rejects update without any confirmation" do + expected_image_url = "https://dummyimage.com/300x300" + expect do + post "/admin/config", params: { site_config: { logo_svg: expected_image_url }, + confirmation: "" } + end.to raise_error ActionController::ParameterMissing end end @@ -329,7 +337,7 @@ RSpec.describe "/admin/config", type: :request do expect do params = { site_config: { shop_url: expected_shop_url }, confirmation: "Incorrect confirmation" } post "/admin/config", params: params - end.to raise_error(Pundit::NotAuthorizedError) + end.to raise_error ActionController::BadRequest expect(SiteConfig.shop_url).not_to eq(expected_shop_url) end @@ -538,8 +546,8 @@ RSpec.describe "/admin/config", type: :request do twitter_hashtag = "#DEVCommunity" params = { site_config: { twitter_hashtag: twitter_hashtag }, confirmation: "Incorrect confirmation" } - it "does not update the twitter hashtag" do - expect { post "/admin/config", params: params }.to raise_error Pundit::NotAuthorizedError + it "does not update the twitter hashtag without the correct confirmation text" do + expect { post "/admin/config", params: params }.to raise_error ActionController::BadRequest end it "updates the twitter hashtag" do