Re: [Rails-core] attr_accessible on some properties + attr_protected on others makes class 'open-by-default'
Sometimes you have a table with a bunch of regular data and one single FK to protect. I don't think forcing users to whitelist that model is a good idea. I prefer Rails to provide both options (three if you count declaring nothing) and leave the judgement of what's appropriate in every situation to the user. -- 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 rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Re: [Rails-core] attr_accessible on some properties + attr_protected on others makes class 'open-by-default'
I'd like to see attr_protected stick around. There are times I'm working with models and I don't want to communicate the15 fields that can be written to but rather the two fields that can't. Best. Mike On Jul 10, 2012, at 1:45 AM, Ryan Bigg radarliste...@gmail.com wrote: For the record: I don't mention attr_protected at all in Rails 3 in Action either. +1 to removing attr_protected. On Tuesday, 10 July 2012 at 11:57 AM, Peter Brown wrote: Just a guy with an opinion weighing in... I would love to see attr_protected removed. The official Rails Guide on security calls attr_accessible A much better way, and I don't think Michael Hartl's popular Ruby on Rails Tutorial even mentions attr_protected. I think it gives people a false sense of security, especially in a large application where it's easy to forget to update it when new fields are added. - Pete On Monday, July 9, 2012 9:38:12 PM UTC-4, Prem Sichanugrist wrote: I personally think we should deprecate attr_protected, and go with whitelisting only (attr_accessible + strong_parameters) route. I think it make more sense from the security standpoint, and all the exploit we have seen. Core teams, wdyt? - Prem -- 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/-/bX4JiC2P5rMJ. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en. -- 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 rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en. -- 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 rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Re: [Rails-core] attr_accessible on some properties + attr_protected on others makes class 'open-by-default'
Yeah, Jay. Your solution won't work with inheritance. By deprecating the attr_protected, you can allow most of the attributes anyway (but seriously seriously seriously discouraged) by do something like: attr_accessible columns - [:created_at, :updated_at] Having attr_accessible and attr_protected together in the same model is just asking for the trouble. You tell the model to whitelist, then you tell it again to blacklist. - Prem On Jul 10, 2012, at 10:03 AM, Rafael Mendonça França rafaelmfra...@gmail.com wrote: Jay, this solution doesn't play nice with inheritance. Rafael Mendonça França http://twitter.com/rafaelfranca https://github.com/rafaelfranca On Tue, Jul 10, 2012 at 10:59 AM, Jay Feldblum yfeldb...@gmail.com wrote: In this type of case, it makes sense either to declare a whitelist or to declare a blacklist. But it doesn't make much sense to declare both of them. Solution #3: ActiveRecord (or ActiveModel) should raise if a class declares both a whitelist and a blacklist of mass-assignable attributes. class Comment attr_accessible: title attr_protected: author_id # raises immediately end Cheers, Jay On Monday, July 9, 2012 6:19:12 PM UTC-4, Uberbrady wrote: (I posted this as a bug in GitHub (https://github.com/rails/rails/issues/7018), but then someone there told me I should post it here, so here it is.) If you set attr_accessible on some properties in an ActiveRecord-descended class, and then attr_protected on others - the class becomes 'default-open' - if any properties are missed or added later, they will be accessible by default to MassAssignment. This undoes the entire point of having put attr_accessible in one's class. Two possible solutions - #1) 'default-closed' - the attr_protected statements will either be ignored, or just used to override attr_accessiblefor a particular property. #2) 'explicit-only' - any attribute accessed in mass-assignment that is not explicitly mentioned in eitherattr_accessible or attr_protected raises a new error - something like MassAssignmentError:AttributeNotExplicitlyDeclared. Maybe even throw an error if the attribute is accessed inany way (mything.whatever=boo; # kerplow! throws error?) though that might perform poorly. Solution #1 is probably fine - accesses to not attr_accessible properties will throw a MassAssignment error under these circumstances anyways. Solution #2 just makes things really explicit, which some might want for some kinds of high-security applications. I found this bug in my own code during the development cycle; I liked putting both attr_accessible andattr_protected in for symmetry and to remind me of my DB schema at the top. Stupid reason, I know. I found that a belongs_to relation was unprotected in that circumstance. -B. -- 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/-/aqdzTPrnZTgJ. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en. -- 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 rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en. -- 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 rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Re: [Rails-core] attr_accessible on some properties + attr_protected on others makes class 'open-by-default'
Prem, What's the conflict with inheritance? Cheers, Jay On Tue, Jul 10, 2012 at 12:29 PM, Prem Sichanugrist sikand...@gmail.comwrote: Yeah, Jay. Your solution won't work with inheritance. By deprecating the attr_protected, you can allow most of the attributes anyway (but seriously seriously seriously discouraged) by do something like: attr_accessible columns - [:created_at, :updated_at] Having attr_accessible and attr_protected together in the same model is just asking for the trouble. You tell the model to whitelist, then you tell it again to blacklist. - Prem On Jul 10, 2012, at 10:03 AM, Rafael Mendonça França rafaelmfra...@gmail.com wrote: Jay, this solution doesn't play nice with inheritance. Rafael Mendonça França http://twitter.com/rafaelfranca https://github.com/rafaelfranca On Tue, Jul 10, 2012 at 10:59 AM, Jay Feldblum yfeldb...@gmail.comwrote: In this type of case, it makes sense either to declare a whitelist or to declare a blacklist. But it doesn't make much sense to declare both of them. Solution #3: ActiveRecord (or ActiveModel) should raise if a class declares both a whitelist and a blacklist of mass-assignable attributes. class Comment attr_accessible: title attr_protected: author_id # raises immediately end Cheers, Jay On Monday, July 9, 2012 6:19:12 PM UTC-4, Uberbrady wrote: (I posted this as a bug in GitHub (https://github.com/rails/** rails/issues/7018 https://github.com/rails/rails/issues/7018), but then someone there told me I should post it here, so here it is.) If you set attr_accessible on some properties in an ActiveRecord-descended class, and then attr_protected on others - the class becomes 'default-open' - if any properties are missed or added later, they will be accessible by default to MassAssignment. This undoes the entire point of having put attr_accessible in one's class. Two possible solutions - #1) 'default-closed' - the attr_protected statements will either be ignored, or just used to override attr_accessiblefor a particular property. #2) 'explicit-only' - any attribute accessed in mass-assignment that is not explicitly mentioned in eitherattr_accessible or attr_**protected raises a new error - something like MassAssignmentError:** AttributeNotExplicitlyDeclared**. Maybe even throw an error if the attribute is accessed in*any* way (mything.whatever=boo; # kerplow! throws error?) though that might perform poorly. Solution #1 is probably fine - accesses to not attr_accessible properties will throw a MassAssignment error under these circumstances anyways. Solution #2 just makes things really explicit, which some might want for some kinds of high-security applications. I found this bug in my own code during the development cycle; I liked putting both attr_accessible andattr_**protected in for symmetry and to remind me of my DB schema at the top. Stupid reason, I know. I found that a belongs_to relation was unprotected in that circumstance. -B. -- 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/-/aqdzTPrnZTgJ. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en. -- 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 rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en. -- 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 rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en. -- 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 rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Re: [Rails-core] attr_accessible on some properties + attr_protected on others makes class 'open-by-default'
On Jul 10, 2012, at 12:29 PM, Prem Sichanugrist wrote: Yeah, Jay. Your solution won't work with inheritance. By deprecating the attr_protected, you can allow most of the attributes anyway (but seriously seriously seriously discouraged) by do something like: attr_accessible columns - [:created_at, :updated_at] Note that this may die in exciting ways if you put it in a model that hasn't been created in the DB yet. (since columns looks at the table metadata) --Matt Jones -- 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 rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
[Rails-core] attr_accessible on some properties + attr_protected on others makes class 'open-by-default'
(I posted this as a bug in GitHub (https://github.com/rails/rails/issues/7018), but then someone there told me I should post it here, so here it is.) If you set attr_accessible on some properties in an ActiveRecord-descended class, and then attr_protected on others - the class becomes 'default-open' - if any properties are missed or added later, they will be accessible by default to MassAssignment. This undoes the entire point of having put attr_accessible in one's class. Two possible solutions - #1) 'default-closed' - the attr_protected statements will either be ignored, or just used to override attr_accessiblefor a particular property. #2) 'explicit-only' - any attribute accessed in mass-assignment that is not explicitly mentioned in eitherattr_accessible or attr_protected raises a new error - something like MassAssignmentError:AttributeNotExplicitlyDeclared. Maybe even throw an error if the attribute is accessed in*any* way (mything.whatever=boo; # kerplow! throws error?) though that might perform poorly. Solution #1 is probably fine - accesses to not attr_accessible properties will throw a MassAssignment error under these circumstances anyways. Solution #2 just makes things really explicit, which some might want for some kinds of high-security applications. I found this bug in my own code during the development cycle; I liked putting both attr_accessible andattr_protected in for symmetry and to remind me of my DB schema at the top. Stupid reason, I know. I found that a belongs_to relation was unprotected in that circumstance. -B. -- 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/-/cKUcUmVToewJ. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Re: [Rails-core] attr_accessible on some properties + attr_protected on others makes class 'open-by-default'
I personally think we should deprecate attr_protected, and go with whitelisting only (attr_accessible + strong_parameters) route. I think it make more sense from the security standpoint, and all the exploit we have seen. Core teams, wdyt? - Prem -- 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 rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Re: [Rails-core] attr_accessible on some properties + attr_protected on others makes class 'open-by-default'
Just a guy with an opinion weighing in... I would love to see attr_protected removed. The official Rails Guide on securityhttp://guides.rubyonrails.org/security.html#countermeasures calls attr_accessible A much better way, and I don't think Michael Hartl's popular Ruby on Rails Tutorial http://ruby.railstutorial.org/ even mentions attr_protected. I think it gives people a false sense of security, especially in a large application where it's easy to forget to update it when new fields are added. - Pete On Monday, July 9, 2012 9:38:12 PM UTC-4, Prem Sichanugrist wrote: I personally think we should deprecate attr_protected, and go with whitelisting only (attr_accessible + strong_parameters) route. I think it make more sense from the security standpoint, and all the exploit we have seen. Core teams, wdyt? - Prem -- 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/-/bX4JiC2P5rMJ. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Re: [Rails-core] attr_accessible on some properties + attr_protected on others makes class 'open-by-default'
For the record: I don't mention attr_protected at all in Rails 3 in Action either. +1 to removing attr_protected. On Tuesday, 10 July 2012 at 11:57 AM, Peter Brown wrote: Just a guy with an opinion weighing in... I would love to see attr_protected removed. The official Rails Guide on security (http://guides.rubyonrails.org/security.html#countermeasures) calls attr_accessible A much better way, and I don't think Michael Hartl's popular Ruby on Rails Tutorial (http://ruby.railstutorial.org/) even mentions attr_protected. I think it gives people a false sense of security, especially in a large application where it's easy to forget to update it when new fields are added. - Pete On Monday, July 9, 2012 9:38:12 PM UTC-4, Prem Sichanugrist wrote: I personally think we should deprecate attr_protected, and go with whitelisting only (attr_accessible + strong_parameters) route. I think it make more sense from the security standpoint, and all the exploit we have seen. Core teams, wdyt? - Prem -- 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/-/bX4JiC2P5rMJ. To post to this group, send email to rubyonrails-core@googlegroups.com (mailto:rubyonrails-core@googlegroups.com). To unsubscribe from this group, send email to rubyonrails-core+unsubscr...@googlegroups.com (mailto:rubyonrails-core+unsubscr...@googlegroups.com). For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en. -- 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 rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
