Safer version of update_attributes

Hi,

I was just working with a model where I am using attr_protected as only certain attributes should be set depending on the context and I was wondering if there is a convention when it comes to setting multiple protected attributes?

e.g. # The rubbish way user = User.find(id) user.name = params[:user][:name] user.email = params[:user][:email] user.save

One attribute per line is a bit tiresome, so you can do: [:name, :email].each do |attr|   user.send("#{attr}=", params[:user][attr]) end

Is there an obvious nicer way I'm missing?

As an alternative I have made a quick monkey patch to AR::Base that allows: user.update_attributes_safely([:name, :email], params[:user])

This just picks the specified keys from the hash, ignoring the protected attributes, but to me seems just as safe? Update_attributes itself could easily be modified too instead of adding another method.

Anyone else do similar?

Regards, Andrew

Andrew France wrote:

Hi,

I was just working with a model where I am using attr_protected as only certain attributes should be set depending on the context and I was wondering if there is a convention when it comes to setting multiple protected attributes?

e.g. # The rubbish way user = User.find(id) user.name = params[:user][:name] user.email = params[:user][:email] user.save

One attribute per line is a bit tiresome, so you can do: [:name, :email].each do |attr|   user.send("#{attr}=", params[:user][attr]) end

Is there an obvious nicer way I'm missing?

class User << ActiveRecord::Base   attr_accessible :name, :email   ...   ... end

Only :name and :email is allowed to be set through mass assignment.

Robert Walker wrote:

Is there an obvious nicer way I'm missing?

class User << ActiveRecord::Base   attr_accessible :name, :email   ...   ... end

Only :name and :email is allowed to be set through mass assignment.

Hum. After taking a second look I may have misread your question. I missed that you were already using attr_protected. I typically use the "while list" method using attr_accessible.

In any case I think I have a better understanding of your question now. Since attr_accessible prevents it's list of attributes from being updated via update_attributes (mass assignment) then you would need to set them individually.

I'm surprised that those values would be coming though the params hash at all (as shown in your example). I would expect that the protected attributes would be set in a more direct manner, but I haven't fully thought this through.

I can't think of the benefit of having :name and :email as protected attributes and then take them from user input. What's the benefit over allowing them though mass assignment? It appears to me that your code is replacing mass assignment with another form of mass assignment.

http://api.rubyonrails.org/classes/ActiveSupport/CoreExtensions/Hash/Slice.html#M001183

Robert Walker wrote: I'm surprised that those values would be coming though the params hash at all (as shown in your example). I would expect that the protected attributes would be set in a more direct manner, but I haven't fully thought this through.

I can't think of the benefit of having :name and :email as protected attributes and then take them from user input. What's the benefit over allowing them though mass assignment? It appears to me that your code is replacing mass assignment with another form of mass assignment.

Part of the problem with the current mass assignment is that it's rather crude - eg you might want an attribute to be changeable at creation time only, or only by users with certain privileges etc, or mass assigned with values from a (trusted) file, distinctions which attr_protected / attr_accessible don't allow you. I think there was talk of overhauling this in rails 3, not sure how far that discussion got

Fred

Frederick Cheung wrote:

Part of the problem with the current mass assignment is that it's rather crude - eg you might want an attribute to be changeable at creation time only, or only by users with certain privileges etc, or mass assigned with values from a (trusted) file, distinctions which attr_protected / attr_accessible don't allow you. I think there was talk of overhauling this in rails 3, not sure how far that discussion got

Thanks. That all makes sense, and I'm looking forward to improvements here.

While this is certainly on-topic based on the subject, I don't see that the OP is attempting any such thing in the examples provided. I still see no benefit to avoiding mass assignment based on what he's doing. If the code had any of the needs you mention here then update_attributes would be lacking.

[:name, :email].each do |attr| user.send("#{attr}=", params[:user][attr]) end

This was what I was referring to when mentioning not seeing the benefit. How is this any safer than update_attributes coupled with attr_accessible or attr_protected?

The benefit for me is that I only want certain attributes to be updated in certain controller contexts. I may have several attributes on the user model that only the root user can update so I would set them to protected in the model and can override it in the controller when the user is root.

That makes sense right?

Andrew

Thanks, I'm aware of the slice method but I would have to unprotect the attributes in order to use it and use slice/reject in every place to prevent malicious values being set. Although of course I could have model.force_update_attributes(params[:user].slice(:blah, :blah)) but I don't that's quite as neat.

Regards, Andrew

Andrew France wrote:

Thanks Robert, I had a look at the documentation for Lockdown. Like similar authorization frameworks (I use declarative_authorization) it seems to support model level access where I can control which users can do what CRUD actions on specific models, but not the actual attributes that are set. I don't really expect auth frameworks to support such a low-level (and rare?) problem.

Cheers for the help.

Andrew