Grape and Protecting Against Mass Assignment Abuse about grape HOT 8 OPEN

First, thank you for writing such a detailed and thorough overview of the problem you’re trying to solve, and code that demonstrates the issue so succinctly.

What would you like the DSL for strong parameters to be?

I think the proposal in #810 is one option.

from grape.

Ok great! Glad to hear that there's some interest in these ideas. And thanks for mentioning #810, that helped me find #1603, #1609, and #2358 which in turn helped me find these PRs:

• #1158 by @hobofan in 2015
• #1606 by @jlfaber in 2017
• #1729 by @seanhandley in 2018

Bummer that these efforts fizzled. There's some variance but generally speaking these approaches look something like this:

params undeclared: :error do
  optional :title, type: String
end

Also supported was a :warn option there in case you didn't want to fail the api call. There were also some ideas of using a method like declared_only but I sorta like how they evolved into an option to params instead. Either way, these approaches are close but not quite what I had in mind.

What I feel is lacking from these approaches are:

• I don't want to fail the api call if undeclared params are passed in
• I do want to raise an exception while an engineer is writing an endpoint with unsafe params
• I want to have a place to define which params are able to be passed to the model

Mitigation Five: add Strong Params to Grape on a fork

After reading and poking around Grape a bit more it occurred to me that I could probably add Strong Params myself pretty easily. So Mitigation Five is done with a branch on my fork. All I did was follow the patterns I saw and:

• create an extension module that implements build_params
• update the main grape.rb file to autoload that new extension
• update the gemspec to add actionpack as a dependency

I'm unsure about that last bit - is adding another piece of Rails to Grape frowned upon?

Spiking on Permitted Params

From here I did a little spiking that I also wanted to share. If I have Strong Params how would I really want to use it? I think I'd want to do something to dry up the list of permitted params and the definition of the params that are safe to use.

So I started by moving the list of permitted params to the model with jonallured/pretend_gravity@a838d97 and then extracted a base class to help with defining a Grape helper method with jonallured/pretend_gravity@c099992. These ideas are only half-baked but maybe help express the direction I'm trying to head.

What do you think? How would you address this stuff? Very open to other thoughts!

from grape.

Based on my level of understanding of this issue, I am not convinced that Strong Params is the best answer for Grape, for these reasons:

• The ability to enforce declared globally on params could be explored further.
• Strong Params would add another gem dependency, which could require more frequent patching of Grape.
• Some of the benefits depend on using Rails models (AR/AM).
• Some benefits for Rails models could be achieved by implementing permitted? on params (false) and the hash returned by declared (true).

from grape.

@dchandekstark We added contract support in #2386. Is that a workable alternative?

from grape.

@dblock I hadn't seen that, but it looks promising, and is perhaps another point against the current proposal -- at least insofar as it wants to replace current features with strong params.

from grape.

@dchandekstark we haven't shipped the contract support yet (2.1.0), will you give it a try? would love your opinion

from grape.

@dchandekstark we haven't shipped the contract support yet (2.1.0), will you give it a try? would love your opinion

Yes, I would like to try the contract approach, mainly due to the nicer encapsulation as compared to the default params. I am wondering if this statement in the doc is accurate though, because, if so, I don't understand it (I am not that familiar with dry-schema, however):

"The latter will define a coercing schema (Dry::Schema.Params). When using the former approach, it's up to you to decide whether the input will need coercing."

from grape.

@jcagarcia ^

from grape.