attr_accessible with an :if option

Hellos

Working on a project I came across the scenario that certain attributes should only be accessible if the model instance is in a certain state. Here’s a completely fake example:

class Car < ActiveRecord::Base
attr_accessible :name, :color

private

def mass_assignment_authorizer(role)
accessible = super
accessible += [:location] if state == :parked
accessible += [:driver, :passengers] if state == :driving
accessible
end
end

Similar to the “if” option of “validates”, a nicer way to do something like this would be:

class Car < ActiveRecord::Base
attr_accessible :name, :color
attr_accessible :driver, :passenger, if: -> {|car| car.state == :driving }
end

Or:

class Car < ActiveRecord::Base
attr_accessible :name, :color
attr_accessible :driver, :passenger, if: ‘driving?’

def driving?
state == :driving
end
end

Would such a feature make sense - or is there maybe an existing mechanism to achieve this without abusing “mass_assignment_authorizer”?

Cheers!

Can you Just Use Ruby? (tm)

$ irb
1.9.3p194 :001 > class Foo
1.9.3p194 :002?> attr_reader :bar if true
1.9.3p194 :003?> end
=> nil
1.9.3p194 :004 > Foo.new.methods.grep(/bar/)
=> [:bar]
1.9.3p194 :005 > class Bar
1.9.3p194 :006?> attr_reader :foo if false
1.9.3p194 :007?> end
=> nil
1.9.3p194 :008 > Bar.new.methods.grep(/foo/)
=> []

Steve,

That won’t work as class evaluation happens once, when the file is loaded, while the state can change during runtime. So the accessibility will not be updated when the state changes.

-Ben

Also, rails 4 will stop using attr_accesible in favor of https://github.com/rails/strong_parameters

Shouldn’t that be handled with validations? I mean you wouldn’t save the model without having a validation to ensure the value is consistent with the state anyways, so why not just take whatever value and let it blow up on validation?

If you want to be nice to the user and silently ignore the attribute instead, that would be a controller concern and strong parameters handles that nicely.

Ahh, good call. I figured there was some reason. :wink:

It feels wrong to use validations for this. Then again, if Rails 4 moves away from attr_accessible towards strong_parameters (which essentially shifts mass assignment protection from the model to the controller), validations will soon be the only way to protect attributes in the model.

As a side note: I’m not sure I like the way this is going in Rails 4. The weblog reads: “We should never have put mass-assignment protection into the model, and many people stopped doing so long ago.” (http://weblog.rubyonrails.org/2012/3/21/strong-parameters/) Why not? It seems perfectly natural to me that a model exposes different attributes under certain circumstances.

Then again, if Rails 4 moves
away from attr_accessible towards strong_parameters

It's not an if, it's already been merged.

Shouldn’t that be handled with validations? I mean you wouldn’t save the model without having a validation to ensure the value is consistent with the state anyways, so why not just take whatever value and let it blow up on validation?

It feels wrong to use validations for this. Then again, if Rails 4 moves away from attr_accessible towards strong_parameters (which essentially shifts mass assignment protection from the model to the controller), validations will soon be the only way to protect attributes in the model.

Why? I think it might be that you are using “attr_accessible” incorrectly. “attr_accessible” and “attr_protected” are unfortunately named. They doesn’t protect your models in anyway EXCEPT when using mass assignment. Even with “attr_accessible :field”, you can still access model.field and set it to whatever value without any error. It does not provide any level of guarantee that I’d consider “protection” for your models. You definitely want a validation rule for this.

As a side note: I’m not sure I like the way this is going in Rails 4. The weblog reads: “We should never have put mass-assignment protection into the model, and many people stopped doing so long ago.” (http://weblog.rubyonrails.org/2012/3/21/strong-parameters/) Why not? It seems perfectly natural to me that a model exposes different attributes under certain circumstances.

Mass assignment itself is perfectly safe. That alone is not the issue. Mass assignment with untrusted data is the problem. And sanitizing user input sounds like a controller (or wherever that data is coming from) concern to me.

It feels wrong to use validations for this. Then again, if Rails 4 moves away from attr_accessible towards strong_parameters (which essentially shifts mass assignment protection from the model to the controller), validations will soon be the only way to protect attributes in the model.
Why? I think it might be that you are using “attr_accessible” incorrectly. “attr_accessible” and “attr_protected” are unfortunately named. They doesn’t protect your models in anyway EXCEPT when using mass assignment. Even with “attr_accessible :field”, you can still access model.field and set it to whatever value without any error. It does not provide any level of guarantee that I’d consider “protection” for your models. You definitely want a validation rule for this.

You’re right, but I don’t want to lock down the models entirely. Some attributes should be protected from mass assignments through the frontend, but there’s still the backend and maybe permitted single attribute modifications through an API.

As a side note: I’m not sure I like the way this is going in Rails 4. The weblog reads: “We should never have put mass-assignment protection into the model, and many people stopped doing so long ago.” (http://weblog.rubyonrails.org/2012/3/21/strong-parameters/) Why not? It seems perfectly natural to me that a model exposes different attributes under certain circumstances.

Mass assignment itself is perfectly safe. That alone is not the issue. Mass assignment with untrusted data is the problem. And sanitizing user input sounds like a controller (or wherever that data is coming from) concern to me.

You’re right again. And after watching Ryan Bates’ 371-strong-parameters cast, I’ll go for strong_parameters. The README of the gem was too spartanic for me to grasp the real idea, but it now makes perfect sense to decouple this from models. And maybe the gem even makes it into Rails 4.

Thanks for discussing this!

As Steve said, it already did :stuck_out_tongue: