Attribute Protection patch

[Will Leinweber]

I added attribute protection to couchrest, along the lines of
activerecord's. The difference is you declare it on each property,
having a separate macro would be redundant, I thought.

It protects from mass assignment from new, update_attributes
(_without_saving), and attributes=

There are two modes of protection
1) Declare accessible properties, assume all the rest are protected
property :name, :accessible => true
property :admin # this will be automatically
protected

2) Declare protected properties, assume all the rest are accessible
property :name # this will not be protected
property :admin, :protected => true

I squished everything down to a single commit that has what's current
on matetti/master as the parent:
http://github.com/will/couchrest/commit/a6a76d8db85d38e68afc4c2a9d33f7f3d81c1410

Or all of the individual commits are at http://github.com/will/couchrest/commits/attribute_protection,
though I think the a6a76d all-as-one commit would be easer to work
with.

I left all of the other behavior the same. I think there's some odd
things going on with initialize vs. update_attribute (initialize will
let you add fields that aren't properties, but update_attribute throws
an error), so I just left it the way it was. Also I think the removal
of _id _rev, and the time stamps could be moved into this protection
mixin, however I think we'd then need an internal initializer that
skips checking for use of class methods like get and the like.

[Matt Aimonetti]

Hi Will,

Thanks for the patch. While reading your description I was wondering how your approach is different from:

   property :slug, :read_only => true

(If I remember correctly, :read_only doesn't create a setter so any methods trying to set a non defined property will fail)

I left all of the other behavior the same. I think there's some odd things going on with initialize vs. update_attribute (initialize will let you add fields that aren't properties, but update_attribute throws an error)

This is done on purpose, a document is a hash and you are free to store more data than defined in the model. However update_attributes is a rails heritage and it's to be used when updating a document after a form POST request. The idea is that you don't want just anybody to inject new entries into your document.

I hope that makes sense.

- Matt

[will leinweber]

Right, the :read_only => true setting prevents a setter from being created, and update_attrbiutes will fail. But a protected attribute with this will still has the setter, and only prevents the mass update in a POST or wherever.

I suppose you could still change an attribute with :read_only using the inherited hash methods, but that doesn't seem right.

The intent for this is attributes that can and do change, just ones you don't want to accidentally let anyone who tampers with post data the ability to change.   

This is done on purpose, a document is a hash and you are free to store more data than defined in the model. However update_attributes is a rails heritage and it's to be used when updating a document after a form POST request. The idea is that you don't want just anybody to inject new entries into your document.

I hope that makes sense.

Yeah, that does make sense. I thought it was probably intentional, but couldn't think of a case from what I've been doing.

- Will

[august...@gmail.com]

Hi Will,

Thanks for this patch... this is one of the main things I have missed
from active record. I'm not maintaining my own fork of couchrest, so
it would be nice if this was merged into Matt's fork.

August

[will leinweber]

On Thu, Dec 3, 2009 at 9:22 AM, Dave Rothlisberger <drot...@gmail.com> wrote:

I've run into a problem using protected attributes with CouchRest: The
protected attributes aren't initialised when loading the document from
CouchDB! […] 

 

Well that's embarrassing! I fixed it here: http://github.com/will/couchrest/commit/b3ed9a8f995b9264bd71952a4b0f9f5fede207dd and sent a pull request to Marcos.

One thing I noticed though, Line 140 of mixins/view.rb (and also suspect 146) never get called in tests. Changing 140 to just return nil makes nothing fail. It was the only change I made not covered by specs. No matter what block I pass into a view, it makes the views not return any rows.

Also Dave, I hope you don't mind me sending this reply to the mailing list too.

-Will