New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bypassing validation #48

Closed
aaronjudd opened this Issue Jan 14, 2014 · 22 comments

Comments

Projects
None yet
3 participants
@aaronjudd

aaronjudd commented Jan 14, 2014

I've been loading some fixture data, where I want some values to be empty when created, but to be validated in all future updates. I was using the collection._collection.insert approach. This is gone now? Is there (or should there be) a parameter that can be passed to skip validation?

@mquandalle

This comment has been minimized.

Contributor

mquandalle commented Jan 14, 2014

myCollection._collection.insert is still supposed to work, on the server only.


Edit: After looking at the source code it seems that you are right, the method to validate document has changed on the server: collection2.js#L148

@aldeed

This comment has been minimized.

Owner

aldeed commented Jan 14, 2014

Yeah, I think it occurred to me that we might need to provide a way to skip validation now, but then I forgot. I'll add an option on the server.

@aldeed

This comment has been minimized.

Owner

aldeed commented Jan 14, 2014

So this is looking to be nearly impossible, at least not without overriding large sections of mongo-livedata server code. C2 now validates in the core insert/update methods on the server, which are eventually used by both client and server, so it keeps the server code simple and consistent. The downside is that there's no way to pass it a validate: false option without overriding all the server-side method calls that comes before that.

I'm sure there's a way, but we'll have to think hard about the best way to restructure the package. In the meantime, you could create a separate app to do the non-validated inserts. (You could connect to the second app from the first and call methods to do the non-validated inserts.) Yes, annoying, but it should work.

Or go back to 0.2.17 for now.

@aaronjudd

This comment has been minimized.

aaronjudd commented Jan 14, 2014

Ok, I'm going to leave 02.17 out of our repo for now, and will look at alternate "no validation" approaches. I like best an approach that has just "validation:false" for collection2, but am going to look at either the MongoDB.connect transport directly or perhaps something from the node library.

@mquandalle

This comment has been minimized.

Contributor

mquandalle commented Jan 14, 2014

Why not using allow/deny rules on the server instead of overwriting insert/update/upsert ?

(And still overwriting those methods on the client, for latency compensation)

@aldeed

This comment has been minimized.

Owner

aldeed commented Jan 14, 2014

The first version of C2 did that, but there are some drawbacks. The most obvious is that people get confused when you mess with their allow/deny paradigm (Meteor behaves differently when there are some defined versus none defined.)

I think we could still go back to using deny even while using Meteor.Collection directly, but I'd like to avoid it.

There are ways to accomplish this, but it involves a lot of overriding which means more chance of breaking with each Meteor release.

@mquandalle

This comment has been minimized.

Contributor

mquandalle commented Jan 14, 2014

From the documentation:

When a client tries to write to a collection, the Meteor server first checks the collection's deny rules. If none of them return true then it checks the collection's allow rules. Meteor allows the write only if no deny rules return true and at least one allow rule returns true.

So if C2 only use a deny rule, I don't understand how it interfere with the user defined rules?

But... are the allow and deny rules checked when you call myCollection._collection.insert() ?

@aldeed

This comment has been minimized.

Owner

aldeed commented Jan 14, 2014

I'm trying to remember exactly, but I think they interfere when the user has no rules defined. Because if you haven't defined any, you expect all inserts to be denied, but as soon as we add a deny rule, it sets _restricted, which means it will now allow all inserts as long as they pass the validation deny function that we added. So that's a potential security issue.

@aldeed

This comment has been minimized.

Owner

aldeed commented Jan 14, 2014

allow/deny are checked for anything you do from the client.

@mquandalle

This comment has been minimized.

Contributor

mquandalle commented Jan 14, 2014

What about adding a falsy allow rule ?

@aldeed

This comment has been minimized.

Owner

aldeed commented Jan 14, 2014

Yes, a falsy allow rule is the workaround that C2 previously used.

@mquandalle

This comment has been minimized.

Contributor

mquandalle commented Jan 14, 2014

So maybe I miss something but,

  • overwritting insert/update/upsert method of the collection object on both the client and the server
  • and define allow/deny rules for server side validation

seems to be a secure - and not hacky - solution.

myCollection._collection.insert will bypassing validation on the server only, like before.

@aldeed

This comment has been minimized.

Owner

aldeed commented Jan 14, 2014

Looking back at the old version of the code, I'd say the only part that seems kind of hacky is autovalues. They need to be generated on the server, and we were able to do so in the deny function and update the doc passed by reference, but that always seemed less than ideal to me.

Still, it worked the old way, and if that's the easiest way to support validate: false, maybe I'll have to switch back.

@aldeed

This comment has been minimized.

Owner

aldeed commented Jan 15, 2014

I have a version working using deny like before, but I want to add more tests before releasing it. Should be within the next 24 hours. (There are actually some more serious bugs with 3.0.0, too, so I want to make sure everything is fixed.)

@aaronjudd

This comment has been minimized.

aaronjudd commented Jan 15, 2014

Looks like I'll just wait then, but more than happy to test or help in anyway. Thanks guys!

@aldeed aldeed closed this in 06ba032 Jan 15, 2014

@aldeed aldeed reopened this Jan 15, 2014

@aldeed

This comment has been minimized.

Owner

aldeed commented Jan 15, 2014

@aaronjudd, it should work now.

Restore ability to do unvalidated inserts and updates on the server. This
can be done with the new validate: false option, or by calling
myCollection._collection.insert (or update) as before. On the client, the
validate: false option will skip client-side validation but not server-side
validation.

@aaronjudd

This comment has been minimized.

aaronjudd commented Jan 15, 2014

Using the _collection method (server side) works fine. I tried the validate false:

  collection.insert item, validate: false, (error, result) ->
    console.log error
    console.log result

Which completes, successfully if I use error handling, but with the (I assume intended) message:
[Error: failed validation]
but the insert worked ok.

However several of the inserts validate:false method received this error

Exception in Mongo write: TypeError: object is not a function
    at packages/mongo-livedata/mongo_driver.js:259
    at runWithEnvironment (packages/meteor/dynamics_nodejs.js:88)

and if I remove error handling, then it's a fatal error.

@aldeed

This comment has been minimized.

Owner

aldeed commented Jan 15, 2014

Hmm? That second error is related to the argument adjustment bug that I was sure I fixed.

You question about unvalidated insert, is that on the client or server?

@aldeed

This comment has been minimized.

Owner

aldeed commented Jan 15, 2014

Oh, I see what the issue is. My fault for coding when tired. :) I'll release a fix shortly.

@aldeed

This comment has been minimized.

Owner

aldeed commented Jan 15, 2014

OK, give the new release a try.

@aaronjudd

This comment has been minimized.

aaronjudd commented Jan 15, 2014

Looks good. The second error I noted I believe is when attempting to insert collections with validate:false where I haven't actually defined a simple-schema, which is understandable, _collection works fine for that use case though, so this looks good now. I have to test more to verify that's absolutely the case (just my CYA, because I didn't really test that thoroughly ;-) )

@aldeed

This comment has been minimized.

Owner

aldeed commented Jan 15, 2014

Oh, right. If you don't define a schema option, then the argument parsing code is skipped, so insert will function like normal insert and will be confused by a second argument that is not a function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment