after insert runs even if insert does not happen

[isAlmogK]

It looks like the after insert runs even if the insert is stopped

I have the following

Organizations.after.insert(function (userId, doc) {
  console.log(userId);
  console.log(doc);
});

and in my organization insert method I have this, which stops the insert however the hook still runs.

var  organizationExist = Organizations.findOne({userId: this.userId});
    if (organizationExist) {
      throw new Meteor.Error(403, TAPi18n.__('organizationExist', {}, currentLang));
    }

[isAlmogK]

Even with the allow deny it runs

Organizations.allow({
  insert: function(userId, doc) {
    var  organizationExist = Organizations.findOne({userId: this.userId});
    if (!organizationExist && userId && Roles.userIsInRole(userId, ['administrator'])) {
      return true
    }
     }
});

I also tried this in the allow but the hook still runs

insert: function(userId, doc) {

    return false;

    // only allow creating an organization if user is logged in and is an administrator
    //return !! userId && Roles.userIsInRole(userId, ['administrator']);
  },

[gwendall]

+1 - May be useful to catch client errors as parameters for the hooks

[matb33]

Are you calling the insert on the server or on the client? If calling from the server, the allow/deny won't take effect.

[isAlmogK]

@matb33 calling the insert from the client client using a method call and running the insert on the server using a meteor method.

[isAlmogK]

I thought the after hook only runs when the mongo doc is inserted?

[matb33]

Unfortunately, the server will trust your insert if called server-side. It makes sense, but it means you may need to rejig your code a bit, i.e. break out your allow/deny logic into separate functions that you can call upon in different situations.

From the meteor docs: Server code is trusted and isn't subject to allow and deny restrictions. That includes methods that are called with Meteor.call — they are expected to do their own access checking rather than relying on allow and deny.

[isAlmogK]

@matb33 right I know this I'm doing my own access checking

 if (organizationExist) {
      throw new Meteor.Error(403, TAPi18n.__('organizationExist', {}, currentLang));
    }

This if check is inside the meteor method before the insert

which stops the mongo insert from happing (the document is never created) so why in the hook still being called

[matb33]

I think I need a clearer picture of what you're doing. Are you calling with a Meteor.call? Can I see in what context your insert is called? Is it right after the throw?

[matb33]

OK, do I have this pieced together accurately?

Shared

Meteor.methods({
  "yourInsert": function (doc) {
    if (!isAllowed(this.userId)) {
      throw new Meteor.Error(403, TAPi18n.__('organizationExist', {}, currentLang));
    }
    Organizations.insert(doc);
  }
});

function isAllowed(userId) {
  var organizationExist = Organizations.findOne({userId: userId});
  return !organizationExist && userId && Roles.userIsInRole(userId, ['administrator']);
}

Server

Organizations.allow({
  insert: function (userId, doc) {
    return isAllowed(userId);
  }
});

Organizations.after.insert(function (userId, doc) {
  console.log("This should not be fired if the organization already exists", userId, doc);
});

Client

Meteor.call("yourInsert", {test: "test"});

[isAlmogK]

Here is everything

Client:

Template.createOrganization.events({
  'submit form': function(e) {
    e.preventDefault();
 Meteor.call('organizationInsert', organization,  TAPi18n.getLanguage(), function(error, result) {
      // display the error to the user and abort
      if (error)
        return throwAppNotification(error.reason, 'danger');
      Router.go('organization');
    });
  }
});

Shared (lib folder, collections)

Meteor.methods({
  organizationInsert: function(organizationAttributes, currentLang) {
    //Checks
    check(Meteor.userId(), String);

    //Checks
    check(organizationAttributes, {
      name: String,
      orgPhoneNumber:String,
      orgEmail:String,
      contactFirstName:String,
      contactLastName:String,
      contactEmail:String,
      contactPhoneNumber:String,
      streetAddress: String,
      city:String,
      stateProvinceRegion:String,
      postalCode: String,
      country: String,
      type: String,
      space: String,
      crop: String,
      measurementOption1: String,
      measurementOption2: String
    });

    check(currentLang, String);

    var loggedInUser = Meteor.user();

    // check user roles
    if (!loggedInUser ||
      !Roles.userIsInRole(loggedInUser,
        ['administrator'])) {
      throw new Meteor.Error(403, TAPi18n.__('accessDenied', {}, currentLang));
    }

    var  organizationExist = Organizations.findOne({userId: this.userId});
    if (organizationExist) {
      throw new Meteor.Error(403, TAPi18n.__('organizationExist', {}, currentLang));
    }
    var  organizationId =  Organizations.insert(organization);
    return {
      _id:  organizationId
    };
  }
});

[matb33]

So on the client, it gets through your checks and calls Organizations.insert then? And then no allow/deny logic on the server end?

[isAlmogK]

Not sure I under there are no client side checks just basic form validation using Parsley.JS. The meteor method is called (this is the collection folder ) inside that I have my checks.

As for allow / deny I'm doing

Organizations.allow({
  insert: function(userId, doc) {
    // only allow creating an organization if user is logged in and is an administrator
    return !! userId && Roles.userIsInRole(userId, ['administrator']);
  },
  update: function(userId, doc) {
    return ownsDocument(userId, doc);
  },
  remove: function(userId, doc) { return ownsDocument(userId, doc); }
}); 

[isAlmogK]

Here is the entire JS file - https://gist.github.com/almogdesign/c2efaa869701d9cb84b3#file-organizations

[matb33]

It really just seems like your checks are leaking through on the client and hitting the insert on the client, subsequently firing on the server unimpeded. The server should also be checking that an organization already exists

[isAlmogK]

@matb33 what's not clear to me that insert never happens it stops once the error is thrown so how does insert hook fire, I thought it only fires when the database doc is created IE inserted?

[matb33]

If that throw really does occur on the client, the remainder of the function is never called and thus the insert is never called. Collection hooks wraps the mutator functions, so something somewhere is calling insert. You might need to dump a stacktrace in the hook callback to help you trace where things may be leaking.

[matb33]

Don't forget that your Meteor method is defined on both server and client. That means that both will run when called from the client. And recall that your allow/deny rules will be ignored when the server is executing the method

[isAlmogK]

This is thrown in the collection JS file which is in the lib -
throw new Meteor.Error(403, TAPi18n.__('accessDenied', {}, currentLang));

which stops the function and when I check the DB there are no new docs so I'm not sure how the insert is being called from somewhere else

[matb33]

Something somewhere is calling insert.

Collection hooks don't tail the oplog or anything fancy like that -- they literally just wrap the original insert methods.

[isAlmogK]

@matb33 I see I thought it was using the oplog, if it just wraps the insert method then use it would fire and would not be able to know when the meteor error is thrown.

It would be nice to have that built in but that wouldn't work as it would still call the hook.

The only real option that does not require using oplog is only running the after hook once the method method is done and has success

[matb33]

The collection hooks wrap the insert function, not the method from Meteor.methods. In your code, if you throw an error within your insertOrganization method before the Organizations.insert is called, then the hooks will not be called, because they are only invoked by code somewhere calling Organizations.insert. Add a console log just before your Organizations.insert to see if it makes it past your checks within your method. If it does make it past, then the last line of defense is the allow/deny checks, but recall that on the server, these will be bypassed.

[isAlmogK]

If that's the case then why is the after hook getting called? I throw the error and the after method stops

[isAlmogK]

I am throwing an error within the insertOrganization method before Organization insert is called but the hook is still getting called?

"if you throw an error within your insertOrganization method before the Organizations.insert is called, then the hooks will not be called"

[matb33]

Right, calling throw will exit out of the current function. Your insert is still getting called somehow.

[isAlmogK]

@matb33 no the insert is not getting called I have doubled checked this and check the DB and I don't see the insert doc. So either there is a bug with the after hook or I'm missing something else.

[isAlmogK]

@matb33 getting something weird when I move the after hook code to a separate JS on the server everything works like expect when the error is thrown the after hook is not called and when everything is ok and the db doc is created it runs.

It seems the issue is when you have the after hook code in the lib folder which both runs on the client and server?

[matb33]

Did you resolve this?

[matb33]

Can you try with 0.7.13? I wonder if what you were experiencing was due to this bug: https://github.com/matb33/meteor-collection-hooks/commit/0f85f2b960615ee1a869d1a79637d37bf6968775

[zimme]

Closing this, please tell me if this wasn't resolved and I'll reopen.