Destroy all and changing schema

[ma...@bitzesty.com]

Hi,
I'm just starting to play with MongoMapper and I ran into an issue
where Model.destroy_all would raise a NoMethodError if I change the
schema of the model. Is this expected? Is there a way to change the
schema on the fly?

stack:
/opt/ruby-enterprise-1.8.6-20090610/lib/ruby/gems/1.8/gems/
mongomapper-0.3.2/lib/mongomapper/embedded_document.rb:159:in `send'
/opt/ruby-enterprise-1.8.6-20090610/lib/ruby/gems/1.8/gems/
mongomapper-0.3.2/lib/mongomapper/embedded_document.rb:159:in
`attributes='
/opt/ruby-enterprise-1.8.6-20090610/lib/ruby/gems/1.8/gems/
mongomapper-0.3.2/lib/mongomapper/embedded_document.rb:158:in
`each_pair'
/opt/ruby-enterprise-1.8.6-20090610/lib/ruby/gems/1.8/gems/
mongomapper-0.3.2/lib/mongomapper/embedded_document.rb:158:in
`attributes='
/opt/ruby-enterprise-1.8.6-20090610/lib/ruby/gems/1.8/gems/
mongomapper-0.3.2/lib/mongomapper/embedded_document.rb:148:in
`initialize'
/opt/ruby-enterprise-1.8.6-20090610/lib/ruby/gems/1.8/gems/
mongomapper-0.3.2/lib/mongomapper/document.rb:175:in `new'
/opt/ruby-enterprise-1.8.6-20090610/lib/ruby/gems/1.8/gems/
mongomapper-0.3.2/lib/mongomapper/document.rb:175:in `find_every'
/opt/ruby-enterprise-1.8.6-20090610/lib/ruby/gems/1.8/gems/
mongomapper-0.3.2/lib/mongomapper/document.rb:175:in `map'
/opt/ruby-enterprise-1.8.6-20090610/lib/ruby/gems/1.8/gems/
mongomapper-0.3.2/lib/mongomapper/document.rb:175:in `find_every'
/opt/ruby-enterprise-1.8.6-20090610/lib/ruby/gems/1.8/gems/
mongomapper-0.3.2/lib/mongomapper/document.rb:33:in `find'
/opt/ruby-enterprise-1.8.6-20090610/lib/ruby/gems/1.8/gems/
mongomapper-0.3.2/lib/mongomapper/document.rb:115:in `destroy_all'

[John Nunemaker]

Hmm. I'll have to think this through. I know what is going on and it
should not cause an error. You are correct in that changes on the fly
should be fine.

The problem now is docs only know how to work with defined keys. If a
key is not defined in the doc, it doesn't know how to handle it and
blows up. It should not. My thought is that it should create a reader
method and just work.

[Scott Motte]

John, if it could do that that would be great! Couchrest does this currently with couchdb. It makes creating custom fields on the fly - or even fields created by the user - super easy. It's a pretty large advantage to schemaless datastores I think, but looks tricky to implement maybe.

Checkout the comments on this post: http://merbist.com/2009/05/17/couchdb-with-couchrest-in-5-minutes/

[John Nunemaker]

My question is this. How do you stop people from just willy nilly injecting params and storing them in your documents? 

I haven't thought the solution all the way through but that pops up as a red flag for me. If someone edits the html of the form and adds all kinds of custom fields, you have to at some point not allow them to do that or they could start cramming your database full of crap. 

Any thoughts on this?

[Keith Hanson]

Wow. Haha. I'd never thought of that. *runs to find some vulnerable Rails CouchDB app* :P

Personally, I honestly don't mind defining my properties/attributes. It helps me remember what the hell the schema looks like down the road, without going and hitting the database later. 

It's nice to be able to have a dynamic schema in CouchDB with little to no barrier of setting up a new field, but for the most part... the inconvenience is approximately two seconds of typing, heh. I'm ok with that ;)

[ma...@bitzesty.com]

Is this really a vulnerability though? I say it's a tradeoff for the
flexibility of being dynamic, I'm sure any important parameters will
be protected (or at least should be)

Matt

On Aug 13, 6:41 pm, Keith Hanson <seraphimrhaps...@gmail.com> wrote:
> Wow. Haha. I'd never thought of that. *runs to find some vulnerable Rails
> CouchDB app* :P
> Personally, I honestly don't mind defining my properties/attributes.
> It helps me remember what the hell the schema looks like down the
> road, without going and hitting the database later.
>
> It's nice to be able to have a dynamic schema in CouchDB with little
> to no barrier of setting up a new field, but for the most part... the
> inconvenience is approximately two seconds of typing, heh. I'm ok with
> that ;)
>

> On Thu, Aug 13, 2009 at 12:37 PM, John Nunemaker <nunema...@gmail.com>wrote:
>
>
>
> > My question is this. How do you stop people from just willy nilly injecting
> > params and storing them in your documents?
> > I haven't thought the solution all the way through but that pops up as a
> > red flag for me. If someone edits the html of the form and adds all kinds of
> > custom fields, you have to at some point not allow them to do that or they
> > could start cramming your database full of crap.
>
> > Any thoughts on this?
>
> > On Aug 13, 2009, at 1:27 PM, Scott Motte wrote:
>
> > John, if it could do that that would be great! Couchrest does this
> > currently with couchdb. It makes creating custom fields on the fly - or even
> > fields created by the user - super easy. It's a pretty large advantage to
> > schemaless datastores I think, but looks tricky to implement maybe.
>
> > Checkout the comments on this post:
> >http://merbist.com/2009/05/17/couchdb-with-couchrest-in-5-minutes/
>

> > On Thu, Aug 13, 2009 at 10:23 AM, John Nunemaker <nunema...@gmail.com>wrote:
>
> >> Hmm. I'll have to think this through. I know what is going on and it
> >> should not cause an error. You are correct in that changes on the fly
> >> should be fine.
>
> >> The problem now is docs only know how to work with defined keys. If a
> >> key is not defined in the doc, it doesn't know how to handle it and
> >> blows up. It should not. My thought is that it should create a reader
> >> method and just work.
>

[Cyril Mougel]

ma...@bitzesty.com a écrit :

> Hi,
> I'm just starting to play with MongoMapper and I ran into an issue
> where Model.destroy_all would raise a NoMethodError if I change the
> schema of the model. Is this expected? Is there a way to change the
> schema on the fly?

Yes you, can I made it. You can change everything on the fly with Ruby
system. Even, Database and collection.

I have some code to made that :

http://gist.github.com/167723

This gist show a system to change all key on your model.

--
Cyril Mougel
http://blog.shingara.fr/

[David James]

On Thu, Aug 13, 2009 at 1:37 PM, John Nunemaker<nune...@gmail.com> wrote:
> My question is this. How do you stop people from just willy nilly injecting
> params and storing them in your documents?

> I haven't thought the solution all the way through but that pops up as a red
> flag for me. If someone edits the html of the form and adds all kinds of
> custom fields, you have to at some point not allow them to do that or they
> could start cramming your database full of crap.

> Any thoughts on this?

That's a controller responsibility, not a model responsibility.

In case you are interested in my criticisms of attr_accessible (they
are related to this discussion I think) check out something I wrote
last year:

http://djwonk.com/blog/2008/05/23/improving-attr_accessible/

[John Nunemaker]

I don't see it as protecting keys you want to protect but more as
allowing someone to just dynamically stuff your database with stuff
you don't want them too.

[scottmotte]

You have a good point John. What use cases do you see this happening?

I could see it happening in an open system - like commenting - where
the user could post using curl and add in 100s or 1000s of random
fields/keys with values. This could potentially overwhelm the database
just like a denial-of-service attack overwhelms the servers. And yes,
that would fill up the database with a bunch of crap, but I think the
issue is the actual 'filling' not the crap since the developer is
still choosing what to display and in that case wouldn't be displaying
the custom fields in the views.

However, I suppose this could also happen in a closed (login/paid)
system where 99% of the users safely and reasonably create custom
string only fields on the fly with some custom textboxes you provide
them - one for the field name and one for the field value (as a
string). There might be one user who causes a raucous. In that case
both the crap and the filling could be the problem - since most likely
the developer is just iterating through those custom fields and
displaying them in the app's views.

Is there some way a developer could defend against this? I'm trying to
think how. How do developers currently defend against it that offer
custom fields in mysql via a relational database or column array or
whatever? Anyone have any ideas?

Maybe the best solution is just as you posed to me a few weeks back
(set a custom field) -
http://groups.google.com/group/mongomapper/browse_thread/thread/cdbf1d78a446fe91/1cc836406bb28c1a?lnk=gst&q=custom+field#1cc836406bb28c1a
- and then let the developer manage those fields and take the risk.
Then again maybe a switch could be setup in mongomapper somehow?

I have no idea, just thinking out loud. Thoughts from everyone?

P.S. - I guess the couchdb with couchrest allows it because it only
allows a string, and I assume they figured the chances of the database
being filled up by a bad user were minimal.




It would certainly be putting a lot more security work in the hand of
the developer.

On Aug 14, 5:12 pm, John Nunemaker <nunema...@gmail.com> wrote:
> I don't see it as protecting keys you want to protect but more as  
> allowing someone to just dynamically stuff your database with stuff  
> you don't want them too.
>

[austinfromboston]

I think the current behavior of throwing exceptions when sent an
undefined key via attributes= is totally appropriate. The datastore
is schema-less but our applications, in general, are not.
I have found this system to be brittle, though, when you _remove_ a
key from your model definition, but do not run any kind of migration
on the database.
Right now, mongomapper uses the same attributes= function for
assigning incoming data from the db.
When data comes in from the db we should use a different assignment
method that silently discards undefined keys. From the application's
point of view, those keys don't exist.
If it's easy, I think these keys should be maintained during updates
to their records. This would allow multiple apps to run on the same
database, each using only the parts of the data they are concerned
with. It's also a hedge against unintentional data loss ( "You
deleted *which* key?" ).

On Aug 13, 10:37 am, John Nunemaker <nunema...@gmail.com> wrote:
> My question is this. How do you stop people from just willy nilly  
> injecting params and storing them in your documents?
>
> I haven't thought the solution all the way through but that pops up as  
> a red flag for me. If someone edits the html of the form and adds all  
> kinds of custom fields, you have to at some point not allow them to do  
> that or they could start cramming your database full of crap.
>
> Any thoughts on this?
>
> On Aug 13, 2009, at 1:27 PM, Scott Motte wrote:
>
>
>
> > John, if it could do that that would be great! Couchrest does this  
> > currently with couchdb. It makes creating custom fields on the fly -  
> > or even fields created by the user - super easy. It's a pretty large  
> > advantage to schemaless datastores I think, but looks tricky to  
> > implement maybe.
>
> > Checkout the comments on this post:http://merbist.com/2009/05/17/couchdb-with-couchrest-in-5-minutes/
>
> > On Thu, Aug 13, 2009 at 10:23 AM, John Nunemaker  
> > <nunema...@gmail.com> wrote:
>
> > Hmm. I'll have to think this through. I know what is going on and it
> > should not cause an error. You are correct in that changes on the fly
> > should be fine.
>
> > The problem now is docs only know how to work with defined keys. If a
> > key is not defined in the doc, it doesn't know how to handle it and
> > blows up. It should not. My thought is that it should create a reader
> > method and just work.
>

[David James]

"allowing someone to just dynamically stuff your database with stuff
you don't want them too."

I don't understand what you mean. Can you be more specific?

What did you think of my specific arguments? To boil it down:
(1) There are times when you want full control over the model. Your
ORM should allow this and not get in your way.
(2) There are times when you don't trust user input and could use some
help at the controller level. I see this as a controller
responsibility myself -- but if you see this as something that belongs
in the model, it should be unobtrusive enough that it doesn't make (1)
difficult.

[ehsanul]

I have to agree with David, it should be a controller responsibility.
It's not so difficult.

Instead of:
Comment.new( params[:comment] )
You'd have to do:
Comment.new( :user => params[:comment][user], :content => params
[:comment][:content] )

Small price to pay if you need the flexibility. However, it's probably
true that most people don't need to change their schema on the fly. So
it makes more sense to introduce a setting, call it unsafe_mode or
something.

After including the MongoMapper::Document, this method would become
available, with default set to 'false'. Then those who need dynamic
key definitions can set "unsafe_mode true", for models in which they
need the extra flexibility. They just have to make sure they don't
allow users to set fields in those models, or ignore any fields they
do try to set in the controller. But users who don't need the
flexibility wouldn't have to bother about that, users setting
different fields would just result in an error.

On Aug 17, 5:39 pm, David James <dja...@sunlightfoundation.com> wrote:
> "allowing someone to just dynamically stuff your database with stuff
> you don't want them too."
>
> I don't understand what you mean. Can you be more specific?
>
> What did you think of my specific arguments? To boil it down:
> (1) There are times when you want full control over the model. Your
> ORM should allow this and not get in your way.
> (2) There are times when you don't trust user input and could use some
> help at the controller level. I see this as a controller
> responsibility myself -- but if you see this as something that belongs
> in the model, it should be unobtrusive enough that it doesn't make (1)
> difficult.
>

[David James]

> So
> it makes more sense to introduce a setting, call it unsafe_mode or
> something.
>
> After including the MongoMapper::Document, this method would become
> available, with default set to 'false'. Then those who need dynamic
> key definitions can set "unsafe_mode true", for models in which they
> need the extra flexibility.

Hmmm. I'm not a big fan of this idea. Generally speaking, I don't like
the kind of design where the behavior of a method (.new or .create in
this case) depends on another setting (e.g. unsafe_mode). It is better
to use method name that have clear names that indicate what they do
without having to check on other 'settings'.

I think it is important to emphasize that there are use cases for
MongoMapper that don't involve the Web and parameter sanitization at
all. It is a persistence framework, not a Web framework. I say: let's
not add functionality where it doesn't belong.

If someone, despite all of this, wants to put in parameter
sanitization, it should not clash or break expectations with the
plain-vanilla versions of 'new' and 'create'.

[Daniel DeLeo]

I'm just getting started with MM, but my use case is to have a trusted application create documents with some arbitrary keys, so I'm not big on "strict mode." For those that require it (which could eventually include me), I'd advocate that the api be something like:

* A validation style class method/macro. This is basically ehansul's idea, but maybe more palatable because it defaults to allowing arbitrary data and uses the same language and a similar API as ActiveRecord.

class Comment

  include MongoMapper::Document

  validates_schema_compliance #or something

  #...

end

* Same as above but using include or extend: ``include MongoMapper::Document::StrictMode'' in addition to MM::Document

* Put the above into one include, so you'd use ``include MongoMapper::StrictModeDocument'' to get document behavior and strict behavior

* A class method to sanitize a hash: Comment.new(Comment.sanitize!(params[:comment]))

* Same as above, but doesn't define the method until you include a Sanitizer module.

* Use different methods to initialize/create sanitized objects: Comment.sanitized(params[:comment]) #=> <Comment:object_id> or something that's both convenient and obvious.

Personally, I like the shorter ones (validation, any flavor, or special constructor methods) the best, but I agree with David in the sense that, for a schema-free DB, locking things down to a schema defined by the app should be considered special or extra behavior and not imposed on everyone just to make web development slightly easier. But with the right API, it should be just as easy for both use cases.

Dan DeLeo

[John Nunemaker]

Anyone with basic knowledge of web forms can save your app's html
locally, put in their own new form elements and submit the form from
their desktop to inject other stuff.

That said, I added dynamic stuff already as I think it is too cool not
to. My main worry was security but now my thought is that there is no
difference between someone stuff a ton of stuff in one static key or a
ton of stuff in several dynamic keys. Either way the security rests
with the developer so why not allow for dynamic stuff.

[John Nunemaker]

There will definitely not be any settings for this. Probably not even
a flag on the model. Everyone should be aware that dynamic keys can be
created and then they just need to have some sort of abuse check/
policy in place to stop people who would attack. Really whether or not
you have dynamic keys you'll need this with any app that gets to a
certain popularity level.

[John Nunemaker]

The sanitize method isn't a bad idea but the name doesn't click with me exactly. Maybe something like one of these?

Comment.accessible(params[:comment])

Comment.only_safe(params[:comment])

Comment.safe(params[:comment])

Maybe even allow changing what is safe based on where you are?

Comment.safe([:name, :age], params[:comment])

Comment.trust([:name, :age], params[:comment])

Those are my thoughts. Keep the discussion going, we'll figure something out.

[austinfromboston]

maybe keeping attributes as the core method and just adding a flag?

Either

Comment.attributes = params[:comment], :create_keys => true/False

Or, if you want to default to the liberal policy

Comment.defined_attributes = params[:comment]

seems like if you want to define the allowed attributes at assignment
you could use filtering

Comment.attributes = params[:comment].reject { |key, val| !
[:name, :age].include?(key) }