Add support for ActiveModel ActionController::Parameters (ActiveModel::ForbiddenAttributesError with active_model 4)

[Maxim-Filimonov]

I'm having issue with grape mounted inside rails 4 application. Whenever I try to use a model backed by activerecord with grape I get ActiveModel::ForbiddenAttributesError. I found a workaround which couples grape to Rails ActionController:

safe_params = ActionController::Parameters.new(params).permit(:registration => [:venue_name, :username, :email, :phone ])
Registration.create(safe_params[:registration])

Am I doing something wrong or it requires PR to fix?

[dblock]

You should build a repro and post it somewhere. I think few people here had a chance to really try Rails 4 + Grape out, so I would imagine this needs fixing or some other changes.

[Maxim-Filimonov]

Right, I have created a test repository
https://github.com/Maxim-Filimonov/grape_404_forbidden_attributes_error
Let me know if you need more info.

[dblock]

I took a look at this. In Rails 4 controllers you're supposed to do this:

 def post_params
    # This says that params[:user] is required, but inside that, only params[:user][:name] and params[:user][:email] are permitted
    # Unpermitted params will be stripped out
    params.require(:user).permit(:name, :email)
  end

Your implementation will do fine and is equivalent to this, albeit not "external" and certainly not pretty.

So following this model Grape would need to allow something similar at the params level. We support this:

params do
  group :user do
    requires :name
    requires :email
  end
end

It's a similar structure that would need to turn into ActionController::Parameters with its permit and require. You could start by trying to implement a helper that could just give you action_controller_compatible_params by examining route.route_params (which has the parsed structure from above). That code would be useful to everyone. The next step would be to integrate it into Grape.

[Maxim-Filimonov]

I looked into the issue a little bit more and it looks like it actually should work without changing grape.
Based on the test from rails active model if a hash doesn't respond to permitted? then forbidden_attributes_protection is not used.
From what I found Hashie::Mash actually responds to any messages with ? or ! at the end.
https://github.com/intridea/hashie/blob/master/lib/hashie/mash.rb#L190
The behaviour was introduced by hashie/hashie@ab75e0c.
Looks like it's indented behaviour but I can't figure out why it was introduced.
hashie/hashie#89 is simular to this one.

[dblock]

Interesting. There's obviously a straightforward monkey patch to prevent this behavior. Do you think there's a better solution?

[Maxim-Filimonov]

If Mash wouldn't respond to "permitted?" monkey patch won't be required.
On the other hand it might be better solution to integrate with strong_parameters. I was thinking what if it returns ActionController::Parameters instead of Hashie::Mash when strong_parameters is present ?

[dblock]

As an external gem, sure. Grape would gladly accomodate an extension point to make it cleaner.

[fsmanuel]

I run in the exact same thing.
What i did is:

helpers do
  def clean_params(params)
    ActionController::Parameters.new(params)
  end
end


post do
  safe_params = clean_params(params).require(:user).permit(:name, :email, :password, :password_confirmation)

  user = User.new(safe_params)
  if user.valid? and user.save
    present user
  else
    error!({errors: user.errors}, 422)
  end
end

It would be great to use the params dsl to wrap strong_params. required, optional define the permited fields, etc.
Validation (maybe related: #433) should be done via strong_params and everyone can catch them.
I have no idea on what extend Hashie::Mash is involved in all that. Is Mash used to generate API documentation? Whould like to know more about it, before i start working on it.

[Maxim-Filimonov]

Mash is just a fancy hash.
Mash, was blocking usage of parameters even if you don't want to filter them without actually implementing a way to filter them. I fixed the issue but still waiting for PR to be merged. If you want the proper support with permit and require its probably should be a separate gem plugin for hashie.When Hashie supports strong_parameters it would be possible to create a gem plugin for grape to allow to reuse DSL for defining parameter constraints as strong_parameters definition.
Quite a lot of work as you see unless I'm missing a shortcut here.
—
Cheers Maxim

On Tue, Jul 23, 2013 at 8:09 AM, Manuel Wiedenmann
notifications@github.com wrote:

I run in the exact same thing.
What i did is:
helpers do
def clean_params(params)
ActionController::Parameters.new(params)
end
end

post do
  safe_params = clean_params(params).require(:user).permit(:name, :email, :password, :password_confirmation)

  user = User.new(safe_params)
  if user.valid? and user.save
    present user
  else
    error!({errors: user.errors}, 422)
  end
end

It would be great to use the params dsl to wrap strong_params. required, optional define the permited fields, etc.
Validation (maybe related: #433) should be done via strong_params and everyone can catch them.

I have no idea on what extend Hashie::Mash is involved in all that. Is Mash used to generate API documentation? Whould like to know more about it, before i start working on it.

Reply to this email directly or view it on GitHub:
#404 (comment)

[fsmanuel]

seems to be too much work ;)
i'll use my helper. i'm more concerned with a nice way to present the errors in a unified format. but this doesn't belong here.

[robertjpayne]

An easier solution for the time being (permits only the parameters you’ve defined anyways!)

helpers do
  def permitted_params
    ActionController::Parameters.new(params).permit(params['route_info'].route_params.keys)
  end
end

Then you can just utilise permitted_params.

[teamon]

You can use declared as described in #484 and make yourself a helper

def permitted_params
  declared(params)
end

[aj0strow]

For people looking for a quick solution:

helpers do
  def declared_params
    declared(params, include_missing: false)
  end
end

params do
  requires :important_field
  optional :unimportant_field
end

post 'objects/:id' do
  object = Object.new(declared_params)
  # save and present
end

Important: if you don't set :include_missing to false, the declared params will include nil fields, which will wipe out data on patch endpoints.

[robjacoby]

The hashie_rails gem seems to resolve this issue

[Maxim-Filimonov]

@robjacoby yep it does forgot to close the issue.

[dm1try]

@robjacoby , @Maxim-Filimonov good information, thx guys!

[shaqq]

@dm1try @Maxim-Filimonov @dblock

Could we re-open this issue? It feels a little hacky to "fix" this by adding hashie-rails. We're also seeing this issue in Napa, so it's not quite just Rails thing (probably ActiveModel or AR).

Also, anyone know what version of Grape this issue started popping up?

[Maxim-Filimonov]

@shaqq hashie-rails is not rails specific anymore. Sorry for confusion, It works with just ActiveSupport
@dblock do we need to rename gem to make it clear or ?

[dblock]

I think that yes, the gem should be renamed or we're going to have confused users for a long time :) Maybe something like hashie-protected-attributes?

[dm1try]

@shaqq as you can see above, it's related to the interaction between Hashie::Mash and ActiveModel

1. ActiveModel raises ForbiddenAttributesError if passed hash responds to #permitted? and return false
2. Hashie::Mash returns false for any undefined attribute if ? method suffix is used. (so permitted? => false)

The hashie-rails gem works fine, just has bad naming.
Btw, @Maxim-Filimonov is ActiveSupport dependency really needed? (seems just hashie is sufficient)