Skip to content

Instantly share code, notes, and snippets.

Embed
What would you like to do?
class PostsController < ActionController::Base
def create
Post.create(post_params)
end
def update
Post.find(params[:id]).update_attributes!(post_params)
end
private
def post_params
params[:post].slice(:title, :content)
end
end
@wxianfeng
Copy link

wxianfeng commented Mar 5, 2012

no anyone use attr_protected?

@filipeamoreira
Copy link

filipeamoreira commented Mar 5, 2012

I really liked this approach. Especially with the added option to do a permission based selection (see follow up comment by @dhh)

@jasonroelofs
Copy link

jasonroelofs commented Mar 5, 2012

@wxianfeng attr_protected, IMO, should be removed from ActiveRecord entirely. Blacklisting is almost always a bad idea, and with the nested form parameters stuff Rails already handles, it will hide very devious security holes. Always use attr_accessible to whitelist what people can change.

@larzconwell
Copy link

larzconwell commented Mar 5, 2012

@jameskilton Agreed attr_protected will make you assume too much about what attributes aren't available.

@norv
Copy link

norv commented Mar 7, 2012

Access control, in particular access from a web form, to the model/database layer, is not a model matter. It's better that attr_accessible needs to be specified from now on, but it's not the end of this issue.

+1 for adnanced, znbailey, original gist.

@atwam
Copy link

atwam commented Mar 8, 2012

I've always wondered if rails should add a Form layer, like symfony does.
Having a separate layer that handles validation, nesting, display, assignment security (aware of current user/role) and update of models isn't too bad, and removes a lot of clutter from models...
The way symfony used to do it : http://www.symfony-project.org/forms/1_4/en/01-Form-Creation

@sbeam
Copy link

sbeam commented Mar 10, 2012

this is great for a simple case, but sometimes slice isn't enough, e.g. nested attributes, and also date/time fields that come in like "posted_on(1i)"=>"2010", "posted_on(2i)"=>"3", "posted_on(3i)"=>"3"

What about a way to have form_for send to the controller which fields actually exist in the form (how to do this in a tamper-proof fashion is a good question, an encrypted serialized hash of the legitimate keys would be needed) - any params that are not actually in the form_for block would be ignored. Of course there would need to be a way to whitelist other fields in case they were legitimately added client-side via JS.

That way you wouldn't have the model concerned about current user roles and authz, or even the controller. If a field exists in the form, then it seems safe to presume the corresponding param is meant to be changed via that form instance, and v/v.

@bkimble
Copy link

bkimble commented Mar 23, 2012

Without the introduction of a deep slice implementation, this approach won't work well with nested attributes/forms. The model will still be the goto spot. A deep slice method in all its recursive glory will be a cpu hog as well :/

@halloffame
Copy link

halloffame commented Apr 4, 2012

I'm not sure if slice supports it, but there are ways to support nested attributes using this technique

params[:post].slice(:title, :content, friends: [ :name, { family: [ :name ] }])

source https://github.com/rails/strong_parameters

@JoshCheek
Copy link

JoshCheek commented Apr 7, 2012

I'd love to see validations also move out of the model.

@sobrinho
Copy link

sobrinho commented Apr 7, 2012

@JoshCheek you can do that using active model ;)

@keithtom
Copy link

keithtom commented Dec 11, 2012

Sweet pattern, and I like the simplicity. One thing though:

If someone forgets, or is unaware of the pattern, then you can potentially expose unwanted mass-assignment.

Perhaps, we can combine the strategies so that you can restrict assignment of certain attributes at the model layer, and then have this pattern act more as a 'mark as safe to mass assign'. This seems like it allows controller endpoints to dictate what inputs are acceptable, while letting the model dictate what attributes require additional checks.

As far as actual implementation, it might look more like

@resource.allow_assignment(:password, :password_confirmation)
@resource.update_attributes(params[:resource])

This of course doesn't have to be in a controller. It is really just the idea of forcing developers to explicitly say, "I really want to mass-assign these attributes here".

Thoughts?

@keithtom
Copy link

keithtom commented Dec 11, 2012

Ah just read the strong_params gem. nevermind...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment