From e204d32f589cac3be9130cc3569c612863a5fd5b Mon Sep 17 00:00:00 2001 From: Paul Bob Date: Tue, 6 Aug 2024 14:44:52 +0300 Subject: [PATCH 01/22] fix: false positive on unpermitted parameters --- lib/avo/resources/base.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/avo/resources/base.rb b/lib/avo/resources/base.rb index 14f6c922a9..4b0f57eb81 100644 --- a/lib/avo/resources/base.rb +++ b/lib/avo/resources/base.rb @@ -461,8 +461,17 @@ def fill_record(record, params, extra_params: [], fields: nil) # Write the user configured extra params to the record if extra_params.present? + # Temporarily disable the action on unpermitted parameters + # The `params` object already had strong parameters applied to it and the base + extra_params have been permitted. + # Here, we only need to handle the extra parameters without reapplying the unpermitted parameters + action_on_unpermitted_parameters = ActionController::Parameters.action_on_unpermitted_parameters + ActionController::Parameters.action_on_unpermitted_parameters = false + # Let Rails fill in the rest of the params record.assign_attributes params.permit(extra_params) + + # Restore the action on unpermitted parameters + ActionController::Parameters.action_on_unpermitted_parameters = action_on_unpermitted_parameters end record From c6bf6daef098622079a6e38802113f898ca3e595 Mon Sep 17 00:00:00 2001 From: Paul Bob Date: Wed, 7 Aug 2024 13:36:55 +0300 Subject: [PATCH 02/22] alternative approach --- lib/avo/resources/base.rb | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/lib/avo/resources/base.rb b/lib/avo/resources/base.rb index 4b0f57eb81..1ea9c9446a 100644 --- a/lib/avo/resources/base.rb +++ b/lib/avo/resources/base.rb @@ -460,21 +460,29 @@ def fill_record(record, params, extra_params: [], fields: nil) end # Write the user configured extra params to the record - if extra_params.present? - # Temporarily disable the action on unpermitted parameters - # The `params` object already had strong parameters applied to it and the base + extra_params have been permitted. - # Here, we only need to handle the extra parameters without reapplying the unpermitted parameters - action_on_unpermitted_parameters = ActionController::Parameters.action_on_unpermitted_parameters - ActionController::Parameters.action_on_unpermitted_parameters = false + assign_extra_attributes(record, params, extra_params) if extra_params.present? - # Let Rails fill in the rest of the params - record.assign_attributes params.permit(extra_params) + record + end - # Restore the action on unpermitted parameters - ActionController::Parameters.action_on_unpermitted_parameters = action_on_unpermitted_parameters + def assign_extra_attributes(record, params, extra_params) + # [:fish_type, :something_else, properties: [], information: [:name, :history], reviews_attributes: [:body, :user_id]] + # becomes + # [:fish_type, :something_else, :properties, :information, :reviews_attributes] + # params at this point are already permited, only need the keys to access them + extra_params_keys = extra_params.flat_map do |element| + if element.is_a?(Hash) + element.keys + else + element + end end - record + # Pick only the extra params + extra_attributes = params.slice(*extra_params_keys) + + # Let Rails fill in the rest of the params + record.assign_attributes extra_attributes end def authorization(user: nil) From 217fbe3b7f4223038af4f9651f242dca63c88b44 Mon Sep 17 00:00:00 2001 From: Paul Bob Date: Wed, 7 Aug 2024 13:42:18 +0300 Subject: [PATCH 03/22] wip --- lib/avo/resources/base.rb | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/lib/avo/resources/base.rb b/lib/avo/resources/base.rb index 1ea9c9446a..1a506f9309 100644 --- a/lib/avo/resources/base.rb +++ b/lib/avo/resources/base.rb @@ -466,20 +466,9 @@ def fill_record(record, params, extra_params: [], fields: nil) end def assign_extra_attributes(record, params, extra_params) - # [:fish_type, :something_else, properties: [], information: [:name, :history], reviews_attributes: [:body, :user_id]] - # becomes - # [:fish_type, :something_else, :properties, :information, :reviews_attributes] - # params at this point are already permited, only need the keys to access them - extra_params_keys = extra_params.flat_map do |element| - if element.is_a?(Hash) - element.keys - else - element - end - end - # Pick only the extra params - extra_attributes = params.slice(*extra_params_keys) + # params at this point are already permited, only need the keys to access them + extra_attributes = params.slice(*flatten_keys(extra_params)) # Let Rails fill in the rest of the params record.assign_attributes extra_attributes @@ -630,6 +619,22 @@ def entity_loader(entity) def record_param @record_param ||= @record.persisted? ? @record.to_param : nil end + + private + + def flatten_keys(array) + # [:fish_type, :something_else, properties: [], information: [:name, :history], reviews_attributes: [:body, :user_id]] + # becomes + # [:fish_type, :something_else, :properties, :information, :reviews_attributes] + array.flat_map do |item| + case item + when Hash + item.keys + else + item + end + end + end end end end From ba92b18aba9ab0983ce729942a8dd32e17200b7b Mon Sep 17 00:00:00 2001 From: Paul Bob Date: Wed, 7 Aug 2024 13:43:14 +0300 Subject: [PATCH 04/22] revert dedicated method --- lib/avo/resources/base.rb | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/lib/avo/resources/base.rb b/lib/avo/resources/base.rb index 1a506f9309..779b75c212 100644 --- a/lib/avo/resources/base.rb +++ b/lib/avo/resources/base.rb @@ -460,18 +460,16 @@ def fill_record(record, params, extra_params: [], fields: nil) end # Write the user configured extra params to the record - assign_extra_attributes(record, params, extra_params) if extra_params.present? + if extra_params.present? + # Pick only the extra params + # params at this point are already permited, only need the keys to access them + extra_attributes = params.slice(*flatten_keys(extra_params)) - record - end - - def assign_extra_attributes(record, params, extra_params) - # Pick only the extra params - # params at this point are already permited, only need the keys to access them - extra_attributes = params.slice(*flatten_keys(extra_params)) + # Let Rails fill in the rest of the params + record.assign_attributes extra_attributes + end - # Let Rails fill in the rest of the params - record.assign_attributes extra_attributes + record end def authorization(user: nil) From da92ccee7e0f4653549d576536f6f1f25b5c614d Mon Sep 17 00:00:00 2001 From: Paul Bob Date: Wed, 7 Aug 2024 13:49:47 +0300 Subject: [PATCH 05/22] shorter comment --- lib/avo/resources/base.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/avo/resources/base.rb b/lib/avo/resources/base.rb index 779b75c212..5de4c57d0e 100644 --- a/lib/avo/resources/base.rb +++ b/lib/avo/resources/base.rb @@ -621,9 +621,9 @@ def record_param private def flatten_keys(array) - # [:fish_type, :something_else, properties: [], information: [:name, :history], reviews_attributes: [:body, :user_id]] + # [:fish_type, information: [:name, :history], reviews_attributes: [:body, :user_id]] # becomes - # [:fish_type, :something_else, :properties, :information, :reviews_attributes] + # [:fish_type, :information, :reviews_attributes] array.flat_map do |item| case item when Hash From 4a6510dba4bb845ea8bac92bd4c12d9675cd2e7a Mon Sep 17 00:00:00 2001 From: Paul Bob Date: Wed, 7 Aug 2024 14:02:06 +0300 Subject: [PATCH 06/22] wip --- app/controllers/avo/actions_controller.rb | 2 +- app/controllers/avo/associations_controller.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/avo/actions_controller.rb b/app/controllers/avo/actions_controller.rb index d01acf2ded..bf3d9eff8d 100644 --- a/app/controllers/avo/actions_controller.rb +++ b/app/controllers/avo/actions_controller.rb @@ -64,7 +64,7 @@ def handle private def action_params - params.permit(:authenticity_token, :resource_name, :action_id, :button, fields: {}) + @action_params ||= params.permit(:authenticity_token, :resource_name, :action_id, :button, fields: {}) end def set_action diff --git a/app/controllers/avo/associations_controller.rb b/app/controllers/avo/associations_controller.rb index f95ed4171f..b78c38bfbd 100644 --- a/app/controllers/avo/associations_controller.rb +++ b/app/controllers/avo/associations_controller.rb @@ -132,7 +132,7 @@ def set_reflection_field end def attachment_id - params[:related_id] || params.require(:fields).permit(:related_id)[:related_id] + params[:related_id] || params.dig(:fields, :related_id) end def reflection_class @@ -200,7 +200,7 @@ def through_reflection? end def additional_params - @additional_params ||= params[:fields].permit(@attach_fields&.map(&:id)) + @additional_params ||= params[:fields].slice(@attach_fields&.map(&:id)) end def set_attach_fields From d95a16409903079c220b0bae5ffa6763e6fb0296 Mon Sep 17 00:00:00 2001 From: Paul Bob Date: Wed, 7 Aug 2024 14:16:40 +0300 Subject: [PATCH 07/22] fix associations --- app/controllers/avo/associations_controller.rb | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/app/controllers/avo/associations_controller.rb b/app/controllers/avo/associations_controller.rb index b78c38bfbd..01d63c6a35 100644 --- a/app/controllers/avo/associations_controller.rb +++ b/app/controllers/avo/associations_controller.rb @@ -200,7 +200,7 @@ def through_reflection? end def additional_params - @additional_params ||= params[:fields].slice(@attach_fields&.map(&:id)) + @additional_params ||= params[:fields].slice(*@attach_fields&.map(&:id)) end def set_attach_fields @@ -214,15 +214,12 @@ def set_attach_fields def new_join_record @resource.fill_record( - @reflection.through_reflection.klass.new, - additional_params.merge( - { - source_foreign_key => @attachment_record.id, - through_foreign_key => @record.id - } + @reflection.through_reflection.klass.new( + source_foreign_key => @attachment_record.id, + through_foreign_key => @record.id ), + additional_params, fields: @attach_fields, - extra_params: [source_foreign_key, through_foreign_key] ) end end From dd8c5e39b852b07f41967f40d8ab9cf084e770d5 Mon Sep 17 00:00:00 2001 From: Paul Bob Date: Wed, 7 Aug 2024 14:55:50 +0300 Subject: [PATCH 08/22] enable raise when unnpermitted parameters on tests --- spec/features/avo/controller_action_spec.rb | 4 ++-- .../avo/custom_fields_in_resource_tools_spec.rb | 11 ++++++++++- spec/rails_helper.rb | 4 ++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/spec/features/avo/controller_action_spec.rb b/spec/features/avo/controller_action_spec.rb index 28cf4148d3..209daf1fdd 100644 --- a/spec/features/avo/controller_action_spec.rb +++ b/spec/features/avo/controller_action_spec.rb @@ -29,14 +29,14 @@ end it "assigns the @widget" do - post :create, params: {user: {name: "Adrian"}} + post :create, params: {user: {first_name: "Adrian"}} assert assigns(:view).new? assert assigns(:view).form? end it "assigns the @widget" do - put :update, params: {id: user.id, user: {name: "Adrian"}} + put :update, params: {id: user.id, user: {first_name: "Adrian"}} assert assigns(:view).edit? assert assigns(:view).form? diff --git a/spec/features/avo/custom_fields_in_resource_tools_spec.rb b/spec/features/avo/custom_fields_in_resource_tools_spec.rb index 3d2226529f..db8e481579 100644 --- a/spec/features/avo/custom_fields_in_resource_tools_spec.rb +++ b/spec/features/avo/custom_fields_in_resource_tools_spec.rb @@ -4,7 +4,7 @@ let(:fish) { create :fish, name: :Salmon } describe "fish information" do - it "sends the params to the model" do + before(:example) do visit avo.edit_resources_fish_path fish expect(page).to have_text "There should be an image of this fish below 🐠" @@ -20,11 +20,20 @@ find('input[name="fish[information][age]"]').set("Fishy age") expect(properties_fields.count).to be 2 + end + + it "raise unnpermited params" do + expect { save }.to raise_error("found unpermitted parameter: :age") + end + + it "sends the params to the model" do expect_any_instance_of(Fish).to receive("fish_type=").with("Fishy type") expect_any_instance_of(Fish).to receive("properties=").with(["Fishy property 1", "Fishy property 2"]) expect_any_instance_of(Fish).to receive("information=").with({name: "Fishy name", history: "Fishy history"}) + ActionController::Parameters.action_on_unpermitted_parameters = :log save + ActionController::Parameters.action_on_unpermitted_parameters = :warning end end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 6594b60af7..08fd27b779 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -49,6 +49,10 @@ # ActiveRecord::Migrator.migrate(File.join(Rails.root, 'db/migrate')) +# Ensure that there are no unpermitted_parameters logs +ActionController::Parameters.action_on_unpermitted_parameters = :raise + + require "support/download_helpers" require "support/request_helpers" From 460cf46a8017d0aeb117b26a63822e7aacc09fd7 Mon Sep 17 00:00:00 2001 From: Paul Bob <69730720+Paul-Bob@users.noreply.github.com> Date: Wed, 7 Aug 2024 14:56:36 +0300 Subject: [PATCH 09/22] Update spec/rails_helper.rb --- spec/rails_helper.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 08fd27b779..ac22e484f5 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -52,7 +52,6 @@ # Ensure that there are no unpermitted_parameters logs ActionController::Parameters.action_on_unpermitted_parameters = :raise - require "support/download_helpers" require "support/request_helpers" From 35cb4de43905a2bbced701b2a92bdb19cd0aecc5 Mon Sep 17 00:00:00 2001 From: Paul Bob Date: Wed, 7 Aug 2024 16:05:20 +0300 Subject: [PATCH 10/22] permit id on action params --- app/controllers/avo/actions_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/avo/actions_controller.rb b/app/controllers/avo/actions_controller.rb index bf3d9eff8d..76bb1ef5dc 100644 --- a/app/controllers/avo/actions_controller.rb +++ b/app/controllers/avo/actions_controller.rb @@ -64,7 +64,7 @@ def handle private def action_params - @action_params ||= params.permit(:authenticity_token, :resource_name, :action_id, :button, fields: {}) + @action_params ||= params.permit(:id, :authenticity_token, :resource_name, :action_id, :button, fields: {}) end def set_action From 6a24b77d9314c221e18e302cfac2cf697c6cc4b5 Mon Sep 17 00:00:00 2001 From: Paul Bob Date: Wed, 7 Aug 2024 16:57:20 +0300 Subject: [PATCH 11/22] wip --- .../custom_fields_in_resource_tools_spec.rb | 22 ++++++++++++- spec/support/avo_helpers.rb | 9 ++++++ spec/system/avo/create_via_belongs_to_spec.rb | 32 ++++++++++++++++--- spec/system/avo/tags_spec.rb | 4 +++ 4 files changed, 62 insertions(+), 5 deletions(-) diff --git a/spec/features/avo/custom_fields_in_resource_tools_spec.rb b/spec/features/avo/custom_fields_in_resource_tools_spec.rb index db8e481579..e8f2fe76ca 100644 --- a/spec/features/avo/custom_fields_in_resource_tools_spec.rb +++ b/spec/features/avo/custom_fields_in_resource_tools_spec.rb @@ -26,7 +26,7 @@ expect { save }.to raise_error("found unpermitted parameter: :age") end - it "sends the params to the model" do + it "sends the params to the model ignoring unpermitted age" do expect_any_instance_of(Fish).to receive("fish_type=").with("Fishy type") expect_any_instance_of(Fish).to receive("properties=").with(["Fishy property 1", "Fishy property 2"]) expect_any_instance_of(Fish).to receive("information=").with({name: "Fishy name", history: "Fishy history"}) @@ -35,5 +35,25 @@ save ActionController::Parameters.action_on_unpermitted_parameters = :warning end + + it "sends all the params to the model" do + expect_any_instance_of(Fish).to receive("fish_type=").with("Fishy type") + expect_any_instance_of(Fish).to receive("properties=").with(["Fishy property 1", "Fishy property 2"]) + expect_any_instance_of(Fish).to receive("information=").with({name: "Fishy name", history: "Fishy history", age: "Fishy age"}) + + with_temporary_class_option( + Avo::Resources::Fish, + :extra_params, + [ + :fish_type, + :something_else, + properties: [], + information: [:name, :history, :age], + reviews_attributes: [:body, :user_id] + ] + ) do + save + end + end end end diff --git a/spec/support/avo_helpers.rb b/spec/support/avo_helpers.rb index 17f4587786..313bcede28 100644 --- a/spec/support/avo_helpers.rb +++ b/spec/support/avo_helpers.rb @@ -8,5 +8,14 @@ def find_component(name) def expect_missing_component(name) expect(page).not_to have_css "[data-component-name=\"#{name.to_s.underscore}\"]" end + + def with_temporary_class_option(_class, option_name, option_value, &block) + previous_value = _class.send(option_name) + _class.send(:"#{option_name}=", option_value) + + block.call + + _class.send(:"#{option_name}=", previous_value) + end end end diff --git a/spec/system/avo/create_via_belongs_to_spec.rb b/spec/system/avo/create_via_belongs_to_spec.rb index e67efeeb05..8ca16446a6 100644 --- a/spec/system/avo/create_via_belongs_to_spec.rb +++ b/spec/system/avo/create_via_belongs_to_spec.rb @@ -28,8 +28,20 @@ expect(page).to have_select('fish_user_id', selected: User.last.name) - click_on 'Save' - sleep 0.2 + with_temporary_class_option( + Avo::Resources::Fish, + :extra_params, + [ + :fish_type, + :something_else, + properties: [], + information: [:name, :history, :age], + reviews_attributes: [:body, :user_id] + ] + ) do + click_on 'Save' + sleep 0.2 + end expect(fish.reload.user).to eq User.last end @@ -86,8 +98,20 @@ expect(page).to have_select('fish_user_id', selected: User.last.name) expect do - click_on 'Save' - sleep 0.2 + with_temporary_class_option( + Avo::Resources::Fish, + :extra_params, + [ + :fish_type, + :something_else, + properties: [], + information: [:name, :history, :age], + reviews_attributes: [:body, :user_id] + ] + ) do + click_on 'Save' + sleep 0.2 + end end.to change(Fish, :count).by(1) expect(Fish.last.user).to eq User.last diff --git a/spec/system/avo/tags_spec.rb b/spec/system/avo/tags_spec.rb index 264815516c..c70c4c56c8 100644 --- a/spec/system/avo/tags_spec.rb +++ b/spec/system/avo/tags_spec.rb @@ -143,8 +143,12 @@ wait_for_tags_to_load(field_value_slot) type(:down, :return) + # TODO: fix "found unpermitted parameter: :user_id-dummy" + # tags field passes a dummy param generated from a fake input which is always unpermitted + ActionController::Parameters.action_on_unpermitted_parameters = :log sleep 0.3 click_on "Run" + ActionController::Parameters.action_on_unpermitted_parameters = :warning end end From 734487aae17bf7c3cef619b5291327015e28b8e9 Mon Sep 17 00:00:00 2001 From: Paul Bob Date: Wed, 7 Aug 2024 16:58:49 +0300 Subject: [PATCH 12/22] lint --- spec/system/avo/create_via_belongs_to_spec.rb | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/spec/system/avo/create_via_belongs_to_spec.rb b/spec/system/avo/create_via_belongs_to_spec.rb index 8ca16446a6..18a1bc5428 100644 --- a/spec/system/avo/create_via_belongs_to_spec.rb +++ b/spec/system/avo/create_via_belongs_to_spec.rb @@ -21,7 +21,7 @@ fill_in 'user_last_name', with: 'LastName' fill_in 'user_password', with: 'password' fill_in 'user_password_confirmation', with: 'password' - click_on 'Save' + click_on "Save" sleep 0.2 end end.to change(User, :count).by(1) @@ -39,7 +39,7 @@ reviews_attributes: [:body, :user_id] ] ) do - click_on 'Save' + click_on "Save" sleep 0.2 end @@ -59,14 +59,14 @@ expect do within('.modal-container') do fill_in 'post_name', with: 'Test post' - click_on 'Save' + click_on "Save" sleep 0.2 end end.to change(Post, :count).by(1) expect(page).to have_select('comment_commentable_id', selected: Post.last.name) - click_on 'Save' + click_on "Save" sleep 0.2 expect(comment.reload.commentable).to eq Post.last @@ -87,7 +87,7 @@ fill_in 'user_last_name', with: 'LastName' fill_in 'user_password', with: 'password' fill_in 'user_password_confirmation', with: 'password' - click_on 'Save' + click_on "Save" sleep 0.2 end end.to change(User, :count).by(1) @@ -109,7 +109,7 @@ reviews_attributes: [:body, :user_id] ] ) do - click_on 'Save' + click_on "Save" sleep 0.2 end end.to change(Fish, :count).by(1) @@ -130,7 +130,7 @@ expect do within('.modal-container') do fill_in 'post_name', with: 'Test post' - click_on 'Save' + click_on "Save" sleep 0.2 end end.to change(Post, :count).by(1) @@ -139,7 +139,7 @@ expect do fill_in 'comment_body', with: 'Test Comment' - click_on 'Save' + click_on "Save" sleep 0.2 end.to change(Comment, :count).by(1) @@ -161,7 +161,7 @@ expect do within('.modal-container') do fill_in 'course_name', with: 'Test course' - click_on 'Save' + click_on "Save" sleep 0.2 end end.to change(Course, :count).by(1) @@ -169,7 +169,7 @@ expect(page).to have_select('course_link_course_id', selected: Course.last.name) expect do - click_on 'Save' + click_on "Save" sleep 0.2 end.to change(Course::Link, :count).by(1) From a9697dc8b238174a3453208bae98da137f5d5cef Mon Sep 17 00:00:00 2001 From: Paul Bob Date: Wed, 7 Aug 2024 16:59:42 +0300 Subject: [PATCH 13/22] lint --- spec/support/avo_helpers.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/support/avo_helpers.rb b/spec/support/avo_helpers.rb index 313bcede28..571f1e705a 100644 --- a/spec/support/avo_helpers.rb +++ b/spec/support/avo_helpers.rb @@ -9,13 +9,13 @@ def expect_missing_component(name) expect(page).not_to have_css "[data-component-name=\"#{name.to_s.underscore}\"]" end - def with_temporary_class_option(_class, option_name, option_value, &block) - previous_value = _class.send(option_name) - _class.send(:"#{option_name}=", option_value) + def with_temporary_class_option(klass, option_name, option_value, &block) + previous_value = klass.send(option_name) + klass.send(:"#{option_name}=", option_value) block.call - _class.send(:"#{option_name}=", previous_value) + klass.send(:"#{option_name}=", previous_value) end end end From 77e892c326ca48da9dfd6dafacc88ae9c1ca9701 Mon Sep 17 00:00:00 2001 From: Paul Bob Date: Wed, 7 Aug 2024 17:00:07 +0300 Subject: [PATCH 14/22] more linting --- spec/support/avo_helpers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/support/avo_helpers.rb b/spec/support/avo_helpers.rb index 571f1e705a..fbe3d52f51 100644 --- a/spec/support/avo_helpers.rb +++ b/spec/support/avo_helpers.rb @@ -10,7 +10,7 @@ def expect_missing_component(name) end def with_temporary_class_option(klass, option_name, option_value, &block) - previous_value = klass.send(option_name) + previous_value = klass.send(option_name) klass.send(:"#{option_name}=", option_value) block.call From cac6c3abcd234f8363459f68b474a680f459d0ed Mon Sep 17 00:00:00 2001 From: Paul Bob Date: Wed, 7 Aug 2024 17:04:35 +0300 Subject: [PATCH 15/22] wip --- spec/features/avo/custom_fields_in_resource_tools_spec.rb | 6 +++--- spec/system/avo/tags_spec.rb | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/spec/features/avo/custom_fields_in_resource_tools_spec.rb b/spec/features/avo/custom_fields_in_resource_tools_spec.rb index e8f2fe76ca..e5c8e47df4 100644 --- a/spec/features/avo/custom_fields_in_resource_tools_spec.rb +++ b/spec/features/avo/custom_fields_in_resource_tools_spec.rb @@ -31,9 +31,9 @@ expect_any_instance_of(Fish).to receive("properties=").with(["Fishy property 1", "Fishy property 2"]) expect_any_instance_of(Fish).to receive("information=").with({name: "Fishy name", history: "Fishy history"}) - ActionController::Parameters.action_on_unpermitted_parameters = :log - save - ActionController::Parameters.action_on_unpermitted_parameters = :warning + with_temporary_class_option(ActionController::Parameters, :action_on_unpermitted_parameters, :log) do + save + end end it "sends all the params to the model" do diff --git a/spec/system/avo/tags_spec.rb b/spec/system/avo/tags_spec.rb index c70c4c56c8..5e5972a138 100644 --- a/spec/system/avo/tags_spec.rb +++ b/spec/system/avo/tags_spec.rb @@ -145,10 +145,10 @@ # TODO: fix "found unpermitted parameter: :user_id-dummy" # tags field passes a dummy param generated from a fake input which is always unpermitted - ActionController::Parameters.action_on_unpermitted_parameters = :log - sleep 0.3 - click_on "Run" - ActionController::Parameters.action_on_unpermitted_parameters = :warning + with_temporary_class_option(ActionController::Parameters, :action_on_unpermitted_parameters, :log) do + sleep 0.3 + click_on "Run" + end end end From 09a6384bb6cc0c2f14fafd1bfa772871e8ab0644 Mon Sep 17 00:00:00 2001 From: Paul Bob Date: Wed, 7 Aug 2024 17:05:22 +0300 Subject: [PATCH 16/22] lint --- .../custom_fields_in_resource_tools_spec.rb | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/spec/features/avo/custom_fields_in_resource_tools_spec.rb b/spec/features/avo/custom_fields_in_resource_tools_spec.rb index e5c8e47df4..8bcbced138 100644 --- a/spec/features/avo/custom_fields_in_resource_tools_spec.rb +++ b/spec/features/avo/custom_fields_in_resource_tools_spec.rb @@ -42,16 +42,16 @@ expect_any_instance_of(Fish).to receive("information=").with({name: "Fishy name", history: "Fishy history", age: "Fishy age"}) with_temporary_class_option( - Avo::Resources::Fish, - :extra_params, - [ - :fish_type, - :something_else, - properties: [], - information: [:name, :history, :age], - reviews_attributes: [:body, :user_id] - ] - ) do + Avo::Resources::Fish, + :extra_params, + [ + :fish_type, + :something_else, + properties: [], + information: [:name, :history, :age], + reviews_attributes: [:body, :user_id] + ] + ) do save end end From 09424f01a1c17405d9597fa62310780df7111055 Mon Sep 17 00:00:00 2001 From: Paul Bob Date: Wed, 7 Aug 2024 17:12:15 +0300 Subject: [PATCH 17/22] fix test --- .../features/avo/uncountable_fish_resource_spec.rb | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/spec/features/avo/uncountable_fish_resource_spec.rb b/spec/features/avo/uncountable_fish_resource_spec.rb index 6077b08013..38bece9ca7 100644 --- a/spec/features/avo/uncountable_fish_resource_spec.rb +++ b/spec/features/avo/uncountable_fish_resource_spec.rb @@ -16,7 +16,19 @@ fill_in "fish_name", with: "Nemo" - save + with_temporary_class_option( + Avo::Resources::Fish, + :extra_params, + [ + :fish_type, + :something_else, + properties: [], + information: [:name, :history, :age], + reviews_attributes: [:body, :user_id] + ] + ) do + save + end nemo = Fish.first From fcb006596ab8a5599df9b6282ff062477af7e2b7 Mon Sep 17 00:00:00 2001 From: Paul Bob Date: Wed, 7 Aug 2024 17:20:59 +0300 Subject: [PATCH 18/22] add age as extra param on default --- spec/dummy/app/avo/resources/fish.rb | 2 +- .../custom_fields_in_resource_tools_spec.rb | 43 +++++++++++++------ .../avo/uncountable_fish_resource_spec.rb | 14 +----- spec/system/avo/create_via_belongs_to_spec.rb | 32 ++------------ 4 files changed, 36 insertions(+), 55 deletions(-) diff --git a/spec/dummy/app/avo/resources/fish.rb b/spec/dummy/app/avo/resources/fish.rb index d6b237e50f..c8a619ac1d 100644 --- a/spec/dummy/app/avo/resources/fish.rb +++ b/spec/dummy/app/avo/resources/fish.rb @@ -3,7 +3,7 @@ class Avo::Resources::Fish < Avo::BaseResource self.search = { query: -> { query.ransack(id_eq: params[:q], name_cont: params[:q], m: "or").result(distinct: false) } } - self.extra_params = [:fish_type, :something_else, properties: [], information: [:name, :history], reviews_attributes: [:body, :user_id]] + self.extra_params = [:fish_type, :something_else, properties: [], information: [:name, :history, :age], reviews_attributes: [:body, :user_id]] self.view_types = -> do if current_user.is_admin? [:table, :grid] diff --git a/spec/features/avo/custom_fields_in_resource_tools_spec.rb b/spec/features/avo/custom_fields_in_resource_tools_spec.rb index 8bcbced138..e16bde06e4 100644 --- a/spec/features/avo/custom_fields_in_resource_tools_spec.rb +++ b/spec/features/avo/custom_fields_in_resource_tools_spec.rb @@ -23,24 +23,29 @@ end it "raise unnpermited params" do - expect { save }.to raise_error("found unpermitted parameter: :age") + # Remove age from extra params + with_temporary_class_option( + Avo::Resources::Fish, + :extra_params, + [ + :fish_type, + :something_else, + properties: [], + information: [:name, :history], + reviews_attributes: [:body, :user_id] + ] + ) do + expect { save }.to raise_error("found unpermitted parameter: :age") + end end it "sends the params to the model ignoring unpermitted age" do expect_any_instance_of(Fish).to receive("fish_type=").with("Fishy type") expect_any_instance_of(Fish).to receive("properties=").with(["Fishy property 1", "Fishy property 2"]) + # Verify that age is not included on the information expect_any_instance_of(Fish).to receive("information=").with({name: "Fishy name", history: "Fishy history"}) - with_temporary_class_option(ActionController::Parameters, :action_on_unpermitted_parameters, :log) do - save - end - end - - it "sends all the params to the model" do - expect_any_instance_of(Fish).to receive("fish_type=").with("Fishy type") - expect_any_instance_of(Fish).to receive("properties=").with(["Fishy property 1", "Fishy property 2"]) - expect_any_instance_of(Fish).to receive("information=").with({name: "Fishy name", history: "Fishy history", age: "Fishy age"}) - + # Remove age from extra params with_temporary_class_option( Avo::Resources::Fish, :extra_params, @@ -48,12 +53,24 @@ :fish_type, :something_else, properties: [], - information: [:name, :history, :age], + information: [:name, :history], reviews_attributes: [:body, :user_id] ] ) do - save + # Don't raise unpermitted parameters error + with_temporary_class_option(ActionController::Parameters, :action_on_unpermitted_parameters, :log) do + save + end end end + + it "sends all the params to the model" do + expect_any_instance_of(Fish).to receive("fish_type=").with("Fishy type") + expect_any_instance_of(Fish).to receive("properties=").with(["Fishy property 1", "Fishy property 2"]) + expect_any_instance_of(Fish).to receive("information=").with({name: "Fishy name", history: "Fishy history", age: "Fishy age"}) + + + save + end end end diff --git a/spec/features/avo/uncountable_fish_resource_spec.rb b/spec/features/avo/uncountable_fish_resource_spec.rb index 38bece9ca7..6077b08013 100644 --- a/spec/features/avo/uncountable_fish_resource_spec.rb +++ b/spec/features/avo/uncountable_fish_resource_spec.rb @@ -16,19 +16,7 @@ fill_in "fish_name", with: "Nemo" - with_temporary_class_option( - Avo::Resources::Fish, - :extra_params, - [ - :fish_type, - :something_else, - properties: [], - information: [:name, :history, :age], - reviews_attributes: [:body, :user_id] - ] - ) do - save - end + save nemo = Fish.first diff --git a/spec/system/avo/create_via_belongs_to_spec.rb b/spec/system/avo/create_via_belongs_to_spec.rb index 18a1bc5428..63b9ebfd5a 100644 --- a/spec/system/avo/create_via_belongs_to_spec.rb +++ b/spec/system/avo/create_via_belongs_to_spec.rb @@ -28,20 +28,8 @@ expect(page).to have_select('fish_user_id', selected: User.last.name) - with_temporary_class_option( - Avo::Resources::Fish, - :extra_params, - [ - :fish_type, - :something_else, - properties: [], - information: [:name, :history, :age], - reviews_attributes: [:body, :user_id] - ] - ) do - click_on "Save" - sleep 0.2 - end + click_on "Save" + sleep 0.2 expect(fish.reload.user).to eq User.last end @@ -98,20 +86,8 @@ expect(page).to have_select('fish_user_id', selected: User.last.name) expect do - with_temporary_class_option( - Avo::Resources::Fish, - :extra_params, - [ - :fish_type, - :something_else, - properties: [], - information: [:name, :history, :age], - reviews_attributes: [:body, :user_id] - ] - ) do - click_on "Save" - sleep 0.2 - end + click_on "Save" + sleep 0.2 end.to change(Fish, :count).by(1) expect(Fish.last.user).to eq User.last From bc4070586ae3daddec8ebe1782d111012c0a05c1 Mon Sep 17 00:00:00 2001 From: Paul Bob Date: Wed, 7 Aug 2024 17:23:01 +0300 Subject: [PATCH 19/22] lint --- spec/features/avo/custom_fields_in_resource_tools_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/features/avo/custom_fields_in_resource_tools_spec.rb b/spec/features/avo/custom_fields_in_resource_tools_spec.rb index e16bde06e4..d7586a24e2 100644 --- a/spec/features/avo/custom_fields_in_resource_tools_spec.rb +++ b/spec/features/avo/custom_fields_in_resource_tools_spec.rb @@ -69,7 +69,6 @@ expect_any_instance_of(Fish).to receive("properties=").with(["Fishy property 1", "Fishy property 2"]) expect_any_instance_of(Fish).to receive("information=").with({name: "Fishy name", history: "Fishy history", age: "Fishy age"}) - save end end From 829827536453624ff6c6278d364b78fc5bb80f5f Mon Sep 17 00:00:00 2001 From: Paul Bob Date: Wed, 7 Aug 2024 17:42:53 +0300 Subject: [PATCH 20/22] rm unnecessary permit --- app/views/avo/home/failed_to_load.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/avo/home/failed_to_load.html.erb b/app/views/avo/home/failed_to_load.html.erb index ffa9a8c2c9..4bfa70f0f0 100644 --- a/app/views/avo/home/failed_to_load.html.erb +++ b/app/views/avo/home/failed_to_load.html.erb @@ -2,7 +2,7 @@ <% classes = 'absolute inset-auto left-1/2 top-1/2 transform -translate-x-1/2 -translate-y-1/2' label = t 'avo.failed_to_load' - src_url = params[:src].present? && !params[:src].starts_with?('http://') ? params.permit(:src).fetch(:src) : nil + src_url = params[:src].present? && !params[:src].starts_with?('http://') ? params[:src] : nil %>
From 227a40a10b38153de6ecb0a8d881cff962da06bf Mon Sep 17 00:00:00 2001 From: Paul Bob Date: Fri, 9 Aug 2024 12:09:22 +0300 Subject: [PATCH 21/22] enforce permitted_params naming --- lib/avo/resources/base.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/avo/resources/base.rb b/lib/avo/resources/base.rb index 5de4c57d0e..882866f32e 100644 --- a/lib/avo/resources/base.rb +++ b/lib/avo/resources/base.rb @@ -445,9 +445,9 @@ def fields_by_database_id .to_h end - def fill_record(record, params, extra_params: [], fields: nil) + def fill_record(record, permitted_params, extra_params: [], fields: nil) # Write the field values - params.each do |key, value| + permitted_params.each do |key, value| field = if fields.present? fields.find { |f| f.id == key.to_sym } else @@ -456,14 +456,14 @@ def fill_record(record, params, extra_params: [], fields: nil) next unless field.present? - record = field.fill_field record, key, value, params + record = field.fill_field record, key, value, permitted_params end # Write the user configured extra params to the record if extra_params.present? # Pick only the extra params # params at this point are already permited, only need the keys to access them - extra_attributes = params.slice(*flatten_keys(extra_params)) + extra_attributes = permitted_params.slice(*flatten_keys(extra_params)) # Let Rails fill in the rest of the params record.assign_attributes extra_attributes From 060af59a13e554c3189f7889525aef8e780f93ec Mon Sep 17 00:00:00 2001 From: Paul Bob Date: Fri, 9 Aug 2024 17:06:00 +0300 Subject: [PATCH 22/22] escape params src --- app/views/avo/home/failed_to_load.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/avo/home/failed_to_load.html.erb b/app/views/avo/home/failed_to_load.html.erb index 4bfa70f0f0..2eaea2ee63 100644 --- a/app/views/avo/home/failed_to_load.html.erb +++ b/app/views/avo/home/failed_to_load.html.erb @@ -2,7 +2,7 @@ <% classes = 'absolute inset-auto left-1/2 top-1/2 transform -translate-x-1/2 -translate-y-1/2' label = t 'avo.failed_to_load' - src_url = params[:src].present? && !params[:src].starts_with?('http://') ? params[:src] : nil + src_url = params[:src].present? && !params[:src].starts_with?('http://') ? CGI.escapeHTML(params[:src]) : nil %>