ValidatedMethod best practice to validate user permission and roles

help
1

I have wondered, which of the following approaches is better to validate user permissions:

Approach 1: Validate user in validate method:

new ValidatedMethod({
	name: methodName,
	validate(doc) {
            // validate document
            // validate userId and that user is in Meteor.users
            // validate user roles against Roles
	},
	run(doc) {
            // process doc...
            // return result
	},
});

Approach 2: Validate user in run method:

new ValidatedMethod({
	name: methodName,
	validate(doc) {
            // validate document
	},
	run(doc) {
            // validate userId and that user is in Meteor.users
            // validate user roles against Roles
            // process doc...
            // return result
	},
});

I am wondering because I have often seen approach 2 when looking into other public packages or repositories but personally I tend to approach 1, so that the run method is not even initialized if user validation fails.

What is your best practice here? Some hidden traps on approach 1, that I should be concerned of?

2

I’d always go with #2, mainly because you should never trust the client.

So, validate on the client to eliminate garbage (and so not do the run). Do all the strict and secure validating on the server, where it can’t be tampered with.

1 Like
3

So validate is executed on the client and run is executed on the server? I thought the whole construct of the methods is executed on client and server?

4

Normally, yes, but you can wrap code in a Meteor.isServer block, or (better) abstract the actual code out into a server-only function (to ensure it’s completely hidden) and provide a client stub to satisfy the function.

5

Is there a way to see / debug this on the client in order to ensure, that nothing that belongs to the client is visible there?

6

I just use Chrome inspector. Obviously, do this in dev mode - minified JS is not easy!

7

Approach 3: Use a mixin

1 Like
8

A good option: https://atmospherejs.com/didericis/permissions-mixin

Using it would be the equivalent of doing the check in run(), but in a reusable manner

1 Like
9

Yeah, use a mixin, that’s what they’re for and they’re quite powerful. We’ve got some mixins to enable automatic logging, roles and attaching this.user to the run() function something like this:

const create = new ValidatedMethod({
  name: 'something.create',
  mixins: [logging, roles, attachUser],
  roles: ['canCreateSomething'],
  validate: new SimpleSchema({
    name: {
      type: String,
      optional: false
    }
  }).validator(),
  run({
    name
  }) {
    // If not logged in, this code is never reached
    if(Meteor.isServer) {
      // Most methods run their code in this block..
      return Something.create({
        name: name,
        creator: this.user.profile.firstname
      }); 
    }
    return; // Return immediately on client but wait for execution
  }
});
  • Adding our attachUser mixin here ensures that we can’t call the method without being logged in as it throws if no user is found. It also attaches the current user in this.user to the run() context.
  • logging is a mixin that logs every method call, and it attaches a this.log() function inside run() which can accrue a log, then display it once execution is done.
  • roles is another mixin which checks for alanning:meteor-roles roles (with some custom stuff). Then we just define the needed role(s) as an array.
  • Client usually just returns immediately unless we want optimistic UI, so we opt-in for that.

Note that the order you compose mixins in matters. [logging, roles, attachUser] is not the same as [attachUser, roles, logging].

4 Likes
10

And if you plan to use the same mix-ins a lot, you can extend validated method to always include those mixins like so:

import { ValidatedMethod } from 'meteor/mdg:validated-method';
import { PermissionsMixin } from 'meteor/didericis:permissions-mixin';

export const SecuredMethod = class SecuredMethod extends ValidatedMethod {
    constructor(methodDefinition) {
        if (Array.isArray(methodDefinition.mixins)) {
            methodDefinition.mixins = methodDefinition.mixins.concat(
                PermissionsMixin
            );
        } else {
            methodDefinition.mixins = [PermissionsMixin];
        }
        super(methodDefinition);
    }
};

You can also extend validated method this way to return promises and add extra common functionality

4 Likes
11

Guys, thanks a lot! I know this is basic stuff and I should have already learned how this works the right way but better late as never!