cult-of-coders / grapher

Grapher: Meteor Collection Joins + Reactive GraphQL like queries
https://atmospherejs.com/cultofcoders/grapher
MIT License
275 stars 53 forks source link

Always call firewall on queries, even when made from server #441

Open Floriferous opened 3 years ago

Floriferous commented 3 years ago

In named queries, the docs recommend you use firewall to perform security checks, but it also suggests you use this place to store the userId on the params object, should you need it later.

This is very problematic because when you call the query from the server, it skips the firewall, therefore potentially breaking queries that depend on the firewall being called.

There also appears to be no way to get the userId back in embody, for example (outside of maybe using Meteor.userId(), but I think that's a bad practice).

I think it should be left to the developer to skip firewall code depending on where the call is made. You could easily write a wrapper that does this.

Another option would be to make sure that userId is available in all places, including embody, embody.$filter and embody.$postFilter.

Lastly, skipping firewalls makes queries much harder to test, since you have to do it from the client, instead of simply calling them from server-side tests with something like this:

export const callWithUserId = (userId, func) => {
  const invocation = new DDPCommon.MethodInvocation({ userId });
  return DDP._CurrentInvocation.withValue(invocation, func);
};