Pull request #1731: enable/disable strong parameters

[Gary Weaver]

Added ability to disable and reenable strong_parameters:
https://github.com/rails/strong_parameters/pull/55

Goal is to allow more hassle-free inclusion of strong_parameters for testing with and without strong_parameters, because I wanted to have that before adding tests for this strong_parameters patch for activeadmin:
https://github.com/gregbell/active_admin/issues/1731

Know this looks suboptimal as it still behaves differently when SP is disabled, but I thought this might better allow disabling without providing broken behavior when reenabled. It may need more tests to flesh out any issues, but this is a first attempt and it has a test to disable and re-enable that appears to work.

Thanks for any feedback.

[Gary Weaver]

Request denied by dhh. See pull request for reasons. Sounds like strong_parameters is living up to its name. It won't be taken down by a measly on/off switch. :p

[Allen Madsen]

Check out https://github.com/thoughtbot/appraisal

Allen Madsen
http://www.allenmadsen.com

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.

To view this discussion on the web visit https://groups.google.com/d/msg/rubyonrails-core/-/4_vA4-Su-mkJ.

To post to this group, send email to rubyonra...@googlegroups.com.
To unsubscribe from this group, send email to rubyonrails-co...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.

[Gary Weaver]

Thanks! Currently we were adding SP support in the patch by letting the developer explictly perform a permit_all_parameters in the ActiveAdmin controller definition. That is not the way we'd like to do it but a more integrated and robust approach should involve more involved changes to AA, and since the developer has to explicitly define that all parameters are being permitted, that works for now even though it is up for debate whether that should be integrated as an option.

So for the appraisal gem- I would set up an appraisal for with SP and without SP and I could just check to see whether StrongParameters constant has been defined or not wherever controllers are defined in the AA tests to know whether to call permit_all_parameters. Sounds like a plan.

Even if SP won't have a kill switch (which I agree there are arguments against), it is requiring temporary workarounds that effectively disable it so that people can use their existing code.

[Pedro Nascimento]

Just a tip: I think CoC would help you here. Why not look for a
specific method defining what parameters are allowed and if there is
one, use it? You would be able to get rid of the
permit_all_parameters.

> https://groups.google.com/d/msg/rubyonrails-core/-/xNoVq3MrtlQJ.

[Gary Weaver]

As per convention, SP's definition of what is allowed in a controller is a hash, which doesn't really fit into the AA definition of a controller from what I see, but I could be wrong.

And AA's definition of a controller is done via a DSL for a controller, not an actual controller.

Hooking into what columns are editable in AA I *think* is going to take some work to work with SP, per cursory review of their code. I could be wrong.

In ActiveAdmin, you can just generate a controller, make changes to the model via migrations, etc. and you still have the ability to perform CRUD UI on the model without having to permit new parameters (fields/attributes) in the controller. So, to have AA perform as it did before, the default would need to be something similar to permit_all_parameters and although we could just make this assumption and say all AA controllers should permit all, we decided to make it a method that said you were permitting all so you could see that in your AA controller definition.

I'm probably misunderstanding though so feel free to point out specifics as to what it could be doing.

Thanks for the feedback!

[Gary Weaver]

Sorry that was a little unclear- by that "perform CRUD UI" part, I was trying to say that AA can automatically provide a CRUD UI for models without having to define permitted attributes- well, not taking into account mass assignment security in Rails 3, that is. I'm actually not sure whether it looks at protected attributes or not. I assume all that would have to change in a Rails 4 version of AA, but right now we just want to make it work with Rails 3 and SP, so whatever gets us there is a hopefully secure and DRY fashion is what we are looking for (or at least it is what at least a handful of us would like to see that are using SP with Rails 3.2.x).

[Pedro Nascimento]

Following AA's DSL, you could have something like this for allowing everything:
allowed_params do |params|
params.permit(User.column_names.map(&:to_sym)
end

If a allowed_params call is not defined in AA's controller, assume
everything is allowed or nothing is allowed, up to what's best for
everyone.

I'm afraid we're way off-topic right now, though. :)

> https://groups.google.com/d/msg/rubyonrails-core/-/YwQvUJphTesJ.