Using ValidatedMethod in 2020

[jkuester]

@rjdavid you are right it should not be required, I thought in the past that you can alter this from the client but since OAuth is capability based it could only be done with the correct tokens and then the user query wouldn’t prevent anything, since the hijacked user would be assumed to be correct. So the permission check can be reduced to:

export const checkPermissions = function (options) {
  // optionally make methods public, if desired
  const exception = options.isPublic
  if (exception) return options

  const runFct = options.run
  options.run = function run (...args) {

    // does the client, that executes the current method
    // have a registered user currently being authenticated?
    const { userId } = this

    if (!userId) {
      throw new Meteor.Error('permissionDenied', 'userNotExists', {userId, name: options.name })
    }

    // only run the "run" function if the user check has passed
    return runFct.apply(this, args)
  }

  return options
}

[denysk]

Guys, I’m not entirely understand the idea behind mixins, how is it better then just normal check inside run()

....
run({ test }) {
    if (!this.userId) {
      throw new Meteor.Error("unauthorized");
    }
    // Do something with test
    return true;
  },
....

[csiszi]

If you have a mixin, you don’t have to repeat that code in every method. This comes especially handy if you update the permission check to something more complex (i.e. you don’t have to update the code in every single method).
You can also create a new class with default mixins, e.g. permission check, external API options, logging.

[denysk]

I see, yes, it makes sense then.

[lucfranken]

@captainn

some problems with ValidatedMethods. For example, if you use SimpleSchema, you might have to run validation twice - once in the validate function, and again to produce auto-values. And since SimpleSchema mutates the original object you pass it, it’s kind of tricky to know what’s happening when you validate with it in the validate function (this is one of the reasons I moved to a different validation strategy). The regular method implementation doesn’t hav

For this tenancy problem it’s really interesting to look into roles: https://github.com/Meteor-Community-Packages/meteor-roles

It has scopes (a scope can for example be a tenant, a project or a shop). So users get access only to certain scopes of data.

You can call those security checks from for example the mix-ins as mentioned above.

[denysk]

And how would you achieve this, below validated method to create a new user. It should only be running from a particulate web page (client), and not from the browsers console, let say for the reason of applying captcha. In other words, I need to make sure this method calls from console or any other places away from the page will fail.

I can only think of a sha256 phrase server will check, and compare with generated on the client (page)

export const registerUser = new ValidatedMethod({
  name: "registerUser",
  isPublic: true,
  mixins: [checkPermissions],
  validate(userData) {
    check(userData, Object);
    check(userData.username, String);
    check(userData.email, String);
    check(userData.password, String);
    check(userData.fullName, String);
  },
  run({ userData }) {
    const userId = Accounts.createUser({
      username: userData.username,
      email: userData.email,
      password: userData.password,
    });
.....

[lucfranken]

You can never ensure that the client is safe. The way your method is called should not be relevant for the safety of your application as the client can always be tempered with.

What goal are you trying to achieve with the prevention to work from console?

[denysk]

Two goals, to integrate captcha and “invitation only” user registration, somehow I should check the invitation link was used.

[lucfranken]

Those checks should be done at the server side. For invitation only for example you could generate a code in a collection which can be used only once. Then you disable the code (by and update) when is has been used.

[peterfkruger]

As @lucfranken says. You don’t need a captcha if your invitation url has a code that will have to match one of your documents, in a, say, UserInvitations collection. If it doesn’t, the request isn’t legitimate. Then you just remove the invitation document as soon as the user has signed up. Also, you can set a temporal validity of the code in the invitation document.

[denysk]

Thanks, exactly what I was thinking of. I would need to use captcha going forward when invitation system is off and normal registrations start.

[rjdavid]

@denysk

I need to make sure this method calls from console or any other places away from the page will fail.

There is no way to guarantee this. All client inputs from any page can be replicated anywhere.

Instead you should focus on what the client/user must have to proceed (like a token as discussed). Then it does not matter where the request came, as long as the user sends that token.

[captainn]

I see an error in here - the signature for validate and run should match - but they do not. Does this method actually work?

If you want to lock it to only the server, just add an Meteor.isServer check in your validate or run bodies, and/or make sure you only include it in your server bundle. You can invoke it using the normal method invocation: Meteor.call('registerUser', { ... });

[denysk]

It does work. But I see what you mean, I’ll change it. Thanks for an advice, certainly will add isServer.

I still have my doubts whenever to use check or SimpleSchema, I can’t find any advantages one vs. another.

[captainn]

The advantage of Simple Schema (or any full object shape validator - JSON Schema/ajv, runtypes, etc.) is that it checks the entire object. Your checks against props object properties is okay, but additional object properties will be tolerated by that validation. If you then ever refactor your code in run to use the entire props object (passing it right in to a document insert for example), there’s a chance things can slip in that you don’t want.

If you do change it, make sure it works. To the best my understanding, it shouldn’t work as is, and if it is, it shouldn’t work if you change it…

[denysk]

Apologies, it would not work, you right, it was a quick rewrite from methods and I fixed it in the code but my question was send before, really does not recall this change

I’ll stick with check for now as it is easier to read, and will keep an eye on any changes.

Would you use (Meteor.isServer) as a best practice anywhere where mongodb inserted/updated?

[captainn]

@denysk

Would you use (Meteor.isServer) as a best practice anywhere where mongodb inserted/updated?

Not necessarily. If I’m using a subscription, I might want insert/update simulated client side. It depends on what I’m trying to do. In most cases, I only run those types of operations server side, and don’t use pub/sub though. I do that by only including those types of methods in the server bundle, and invoking them with Meteor.call - however, if you are working with a team, it’s best to protect yourself, and a quick if (Meteor.isServer) can make it super clear for anyone else on the team that it’s only meant to run on the server.

(And there are always at least 2 developers - you and you in 6 months. )

[rjdavid]

@captainn

And there are always at least 2 developers - you and you in 6 months.

Something that I can add on my dev slides