attr_accessible with an :if option

[svoop]

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!

[Steve Klabnik]

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/)
=> []

[Ben Woosley]

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

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.

To post to this group, send email to rubyonra...@googlegroups.com.
To unsubscribe from this group, send email to rubyonrails-co...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.

[Luís Ferreira]

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

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To post to this group, send email to rubyonra...@googlegroups.com.
To unsubscribe from this group, send email to rubyonrails-co...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.

Cumprimentos,

Luís Ferreira

[Steve Klabnik]

Ahh, good call. I figured there was some reason. ;)

[Godfrey Chan]

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.

Sent from my phone

[svoop]

On Monday, October 1, 2012 11:00:07 PM UTC+2, Godfrey Chan wrote:

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.

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.

[Steve Klabnik]

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

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

[Godfrey Chan]

On 2012-10-01, at 5:27 PM, svoop wrote:

On Monday, October 1, 2012 11:00:07 PM UTC+2, Godfrey Chan wrote:

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.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.

To view this discussion on the web visit https://groups.google.com/d/msg/rubyonrails-core/-/IYq-qooSH8YJ.

[svoop]

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!

[Godfrey Chan]

On 2012-10-02, at 6:46 AM, svoop wrote:

And maybe the gem even makes it into Rails 4.

As Steve said, it already did :P