Skip to content
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

Don't run autoValue or defaultValue when field will be denied #4

Open
tylercrompton opened this issue Aug 26, 2017 · 4 comments
Open

Comments

@tylercrompton
Copy link

tylercrompton commented Aug 26, 2017

Consider the following example:

const postSchema = new SimpleSchema({
  title: {
    type: String
  },
  content: {
    type: String
  },
  createdAt: {
    type: Date,
    autoValue: function () {
      return new Date();
    },
    denyUpdate: true
  }
});

const Posts = new Mongo.Collection('posts');
Posts.attachSchema(postSchema);

const postId = Posts.insert({title: 'Hello', content: 'World'});
Posts.update({_id: postId}, {content: 'Earth'});

The last line causes an error that vaguely says the the createdAt property is invalid, which leads one to believe that the issue relates to the type, value, or something similar--not the implicit addition of the property. I spent about an hour trying to figure out why my data wasn't passing validation.

One workaround is to construct the autoValue method as follows:

function () {
  if (this.isInsert) {
    return new Date();
  }
}

However, if I wanted to do it that way, I might as well just throw an error when the property was explicitly defined outside of an insertion operation, which obviously defeats the purpose of this package.

Personally, I wouldn't expect autoValue to be called during an update operation when denyUpdate is set to true. I can't conceive of any reasonable use cases in which such behavior would be desirable.

@achirkof
Copy link

Hi @tylercrompton ! I have same problem. And also spent an hour trying to understand what's wrong.

@aldeed
Copy link
Collaborator

aldeed commented Nov 22, 2017

Since this is an add-on package, we can't prevent autoValue from running. The workaround of checking isInsert is the right way to do it, but it doesn't solve the issue for defaultValue. I will leave this open in case anyone can figure out an elegant solution, but this is one of the reasons that I don't personally use or recommend using denyInsert or denyUpdate. There are better ways to handle security.

@aldeed aldeed changed the title Unexpected behavior when using autoValue function Don't run autoValue or defaultValue when field will be denied Nov 22, 2017
@crapthings
Copy link

@aldeed

if dont write allow deny rule in methods

is it good to do insert update remove by checking permission inside before hook ?

@aldeed
Copy link
Collaborator

aldeed commented Nov 30, 2017

@crapthings You could use before hooks or a package like https://github.com/reactioncommerce/meteor-security.

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

No branches or pull requests

4 participants