kitchen-cli icon indicating copy to clipboard operation
kitchen-cli copied to clipboard

Collection Roles forces using Server Methods for Insert/Update/etc?

Open vnevoa opened this issue 5 years ago • 1 comments

Hi.

Not sure if I'm doing things wrong, or if this is a bug or a misunderstanding.

I recently revived a meteor-kitchen project that had been dormant for about 2 years (but functional), and some things are mysteriously broken now after upgrading to latest meteor and kitchen.

My collection definition inside the json recipe is: { "name": "Groups", "roles_allowed_to_insert": ["manager"], "after_insert_code": " console.log('INSERT GROUP '+doc.Cd); ", },

And then, somewhere inside the app, in a component of a page, with the proper user profile (a "manager") logged in, I try running this in a state that goes into the Insert branch: var group = Groups.findOne({Cd: doc.Cd}); if(group) { Groups.update(group._id, { $set: doc }); } else { Groups.insert(doc); }

And the result is logged in the client console by the meteor code:

insert failed: Access denied

I've debugged the server code and yes, the Groups.allow() function is being called with a negative return.

This is unexpected because I'm definitely running with the user profile that should be able to insert into the collection. Am I wrong to assume this? It worked last time.

Checking the code generated by kitchen, I see the file server/collections/groups.js with: Groups.allow({ insert: function (userId, doc) { return false; }, });

This pretty much prevents any insert attempt from the client side, no matter which profile does it. However, the file server/methods/groups.js does have this: Meteor.methods({ "groupsInsert": function(data) { if(!Groups.userCanInsert(this.userId, data)) { throw new Meteor.Error(403, "Forbidden."); } return Groups.insert(data); }, });

And the file both/collections.groups.js has this: this.Groups = new Mongo.Collection("groups"); this.Groups.userCanInsert = function(userId, doc) { return Users.isInRoles(userId, ["manager"]); };

So, it becomes apparent that meteor-kitchen now wants us to use server methods instead of direct collection methods. Is that it? This is not a good thing in my view, because server methods tend to need more code for error checking and recovery, and for most usages I don't think it is necessary as long as allow() is properly defined.

Why doesn't Collection.allow() do the user role checking directly?

Thanks.

vnevoa avatar Sep 14 '19 12:09 vnevoa

I have found the answer to my own question: allow/deny is deprecated because it is insecure. https://guide.meteor.com/security.html#allow-deny

vnevoa avatar Sep 20 '19 15:09 vnevoa