From c0b2c93359e8f7dd82a22391478eed423fbc244a Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Sat, 18 Jan 2025 20:03:14 -0500 Subject: [PATCH] Support Strong Parameters Add support for handling [attribute sanitization][] through [Strong Parameters][]-compliant classes like [ActionController::Parameters][]. The implementation draws inspiration from built-in `rails` cases like [ActiveModel::ForbiddenAttributesProtection][]. To test this behavior, add the `StrongParameters` class to implement `#permitted?`, `#permit!`, and `#to_hash`. [attribute sanitization]: https://guides.rubyonrails.org/active_model_basics.html#attribute-assignment [Strong Parameters]: https://edgeapi.rubyonrails.org/classes/ActionController/StrongParameters.html [ActionController::Parameters]: https://edgeapi.rubyonrails.org/classes/ActionController/Parameters.html#constant-EMPTY_HASH [ActiveModel::ForbiddenAttributesProtection]: https://github.com/rails/rails/blob/v8.0.0.1/activemodel/lib/active_model/forbidden_attributes_protection.rb#L23-L30 --- lib/active_resource.rb | 1 + lib/active_resource/base.rb | 5 ++++- .../forbidden_attributes_protection.rb | 19 +++++++++++++++++ test/abstract_unit.rb | 1 + test/cases/base/load_test.rb | 10 +++++++++ test/cases/finder_test.rb | 13 ++++++++++++ test/strong_parameters.rb | 21 +++++++++++++++++++ 7 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 lib/active_resource/forbidden_attributes_protection.rb create mode 100644 test/strong_parameters.rb diff --git a/lib/active_resource.rb b/lib/active_resource.rb index 8e4f34ecf4..c04074caff 100644 --- a/lib/active_resource.rb +++ b/lib/active_resource.rb @@ -39,6 +39,7 @@ module ActiveResource autoload :Callbacks autoload :Connection autoload :CustomMethods + autoload :ForbiddenAttributesProtection autoload :Formats autoload :HttpMock autoload :Schema diff --git a/lib/active_resource/base.rb b/lib/active_resource/base.rb index 29376f081d..b1d2d9843d 100644 --- a/lib/active_resource/base.rb +++ b/lib/active_resource/base.rb @@ -1041,6 +1041,7 @@ def all(*args) end def where(clauses = {}) + clauses = sanitize_for_mass_assignment(clauses) raise ArgumentError, "expected a clauses Hash, got #{clauses.inspect}" unless clauses.is_a? Hash find(:all, params: clauses) end @@ -1471,7 +1472,7 @@ def load(attributes, remove_root = false, persisted = false) raise ArgumentError, "expected attributes to be able to convert to Hash, got #{attributes.inspect}" end - attributes = attributes.to_hash + attributes = sanitize_for_mass_assignment(attributes).to_hash @prefix_options, attributes = split_options(attributes) if attributes.keys.size == 1 @@ -1720,7 +1721,9 @@ def method_missing(method_symbol, *arguments) # :nodoc: class Base extend ActiveModel::Naming extend ActiveResource::Associations + extend ForbiddenAttributesProtection + include ForbiddenAttributesProtection include Callbacks, CustomMethods, Validations include ActiveModel::Conversion include ActiveModel::Serializers::JSON diff --git a/lib/active_resource/forbidden_attributes_protection.rb b/lib/active_resource/forbidden_attributes_protection.rb new file mode 100644 index 0000000000..40b3a1be70 --- /dev/null +++ b/lib/active_resource/forbidden_attributes_protection.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require "active_model/forbidden_attributes_protection" + +module ActiveResource + class ForbiddenAttributesError < ActiveModel::ForbiddenAttributesError + end + + module ForbiddenAttributesProtection + include ActiveModel::ForbiddenAttributesProtection + + private + def sanitize_for_mass_assignment(attributes) + super + rescue ActiveModel::ForbiddenAttributesError + raise ForbiddenAttributesError + end + end +end diff --git a/test/abstract_unit.rb b/test/abstract_unit.rb index 71136d9516..72d655d919 100644 --- a/test/abstract_unit.rb +++ b/test/abstract_unit.rb @@ -11,6 +11,7 @@ require "active_support" require "active_support/test_case" require "setter_trap" +require "strong_parameters" require "active_support/logger" require "base64" diff --git a/test/cases/base/load_test.rb b/test/cases/base/load_test.rb index 94e2915228..c89dfa4fee 100644 --- a/test/cases/base/load_test.rb +++ b/test/cases/base/load_test.rb @@ -127,6 +127,16 @@ def test_load_object_with_implicit_conversion_to_hash assert_equal @matz.stringify_keys, @person.load(FakeParameters.new(@matz)).attributes end + def test_load_object_with_unpermitted_strong_parameters + params = StrongParameters.new(@matz) + assert_raises(ActiveResource::ForbiddenAttributesError) { @person.load(params) } + end + + def test_load_object_with_permitted_strong_parameters + params = StrongParameters.new(@matz).tap(&:permit!) + assert_equal @matz.stringify_keys, @person.load(params).attributes + end + def test_after_load_attributes_are_accessible assert_equal Hash.new, @person.attributes assert_equal @matz.stringify_keys, @person.load(@matz).attributes diff --git a/test/cases/finder_test.rb b/test/cases/finder_test.rb index 5aea5496ab..9b36ab739c 100644 --- a/test/cases/finder_test.rb +++ b/test/cases/finder_test.rb @@ -63,6 +63,19 @@ def test_where_with_clauses assert_kind_of StreetAddress, addresses.first end + def test_where_clause_with_unpermitted_params + params = StrongParameters.new(person_id: "1") + error = assert_raises(ActiveResource::ForbiddenAttributesError) { StreetAddress.where(params) } + assert_kind_of ActiveModel::ForbiddenAttributesError, error + end + + def test_where_clause_with_permitted_params + params = StrongParameters.new(person_id: "1").tap(&:permit!) + addresses = StreetAddress.where(params) + assert_equal 1, addresses.size + assert_kind_of StreetAddress, addresses.first + end + def test_where_with_clause_in ActiveResource::HttpMock.respond_to { |m| m.get "/people.json?id%5B%5D=2", {}, @people_david } people = Person.where(id: [2]) diff --git a/test/strong_parameters.rb b/test/strong_parameters.rb new file mode 100644 index 0000000000..52dbd68c73 --- /dev/null +++ b/test/strong_parameters.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class StrongParameters + def initialize(parameters = {}) + @parameters = parameters + @permitted = false + end + + def permitted? + @permitted + end + + def permit! + @permitted = true + end + + def to_hash + @parameters.to_hash + end + alias to_h to_hash +end