Protector's strong_parameters mass attributes filter doesn't run when creating

[alexmchale]

I'm having some trouble with the Protector + Strong Parameters + Inherited Resources combination.

See this chunk of code from Protector's strong_parameters integration: https://github.com/inossidabile/protector/blob/master/lib/protector/adapters/active_record/strong_parameters.rb#L15

It's preventing sanitize_for_mass_assignment from running on create.

I've got a basic model and controller here to demonstrate the problem.

Controller: https://github.com/alexmchale/atomic_digester/blob/master/app/controllers/feeds_controller.rb

Model: https://github.com/alexmchale/atomic_digester/blob/master/app/models/feed.rb

Failed spec: https://github.com/alexmchale/atomic_digester/blob/master/spec/controllers/feeds_controller_spec.rb

Failures: 1) FeedsController#create creates a new feed with valid parameters Failure/Error: post :create, feed: { ActiveModel::ForbiddenAttributesError: ActiveModel::ForbiddenAttributesError

./spec/controllers/feeds_controller_spec.rb:12:in `block (3 levels) in <top (required)>'

It seems to me that this should be working, with Protector's strong_parameters and inherited_resources integration taking care of preventing the forbidden attributes exception.

Do you see something I've done wrong here? Should the !new_record? clause be removed from Protector's strong_parameters integration?

[inossidabile]

Creation and initiation of objects is handled on another level (within Relation). It might be an issue of integration between Protector and IR. @grindars how do you restrict an object during create?

[inossidabile]

@alexmchale also can you please define create method explicitly as IR allows and tell me if it even gets restricted?

[grindars]

Protector::IR restricts entities by extending end_of_association_chain. It should work with the default create implementation as-is.

[inossidabile]

And the tests for Protector::IR show the same. Yeah, @alexmchale, please show us what gets into controller and how it gets restricted.

[alexmchale]

@inossidabile As far as I can see, there is no test that verifies that #create works with protector-inherited_resources. The tests surround #index, #show, and #new.

[alexmchale]

The resource is instantiated using #new, and is then properly restricted by Protector::InheritedResources here:

https://github.com/grindars/protector-inherited_resources/blob/master/lib/protector/inherited_resources/instance_methods.rb#L23

But it then proceeds to be run through sanitize_for_mass_assignment and is not sanitized due to:

https://github.com/inossidabile/protector/blob/master/lib/protector/adapters/active_record/strong_parameters.rb#L19

[alexmchale]

The comment in Protector says:

# We check only for updation here since the creation will be handled by relation
# (see Protector::Adapters::ActiveRecord::Relation#new_with_protector and
# Protector::Adapters::ActiveRecord::Relation#create_with_protector)

But I don't see anything in those methods that indicate that they handle mass assignment filtering.

[inossidabile]

As far as I can see, there is no test that verifies that #create works with protector-inherited_resources. The tests surround #index, #show, and #new.

Ok, it has been added. Everything works properly.

But it then proceeds to be run through sanitize_for_mass_assignment and is not sanitized due to: But I don't see anything in those methods that indicate that they handle mass assignment filtering.

At that point it is already sanitized by:

https://github.com/inossidabile/protector/blob/master/lib/protector/adapters/active_record/relation.rb#L92 https://github.com/inossidabile/protector/blob/master/lib/protector/adapters/active_record/relation.rb#L106 https://github.com/inossidabile/protector/blob/master/lib/protector/adapters/active_record/relation.rb#L117

[inossidabile]

Is that atomic_digester an example of application that actually shows the bug in the wild?

[alexmchale]

The app atomic_digester isn't really "in the wild", it's just a toy I'm playing with to learn more about protector and inherited_resources.

What I'm seeing is that the object isn't being instantiated through an ActiveRecord::Relation#build, but rather through ActiveRecord::Associations::CollectionAssociation#build_record.

protector does patch this method to add restrict!, but it does no strong_parameters filtering.

[alexmchale]

I'm wondering if it doesn't have something to do with the logic fork here:

https://github.com/rails/rails/blob/master/activerecord/lib/active_record/associations/collection_association.rb#L126

Rails decides whether to call build_record or build based on whether the input is an array or hash.