Need for auth hook for a method

[johanbrook]

This might be redundant and dilute the API of validated-method, but I was thinking of a common, repetitive case when writing Meteor methods: doing user authentication.

We always end up doing this:

Utils.checkAuth = (userId) => {
  if(!userId) throw new Meteor.Error('not-authorized', 'You need to log in!');
};
// ..
'domain/method': function(args) {
  check(args, /* ... */);
  Utils.checkAuth(this.userId);
  // ..
}

…or having to do custom auth and user role checking stuff on top of the simple checkAuth function.

So I was just thinking: instead of having an auth checking call in the top of each method body, would it be nicer to bake this into validated-method, just like the validate feature?

Like:

const method = new ValidatedMethod({
  name, // DDP method name
  validate, // argument validation
  authenticate, // user authentication function
  run // method body
});

[stubailo]

At first we actually had this, but it turned out that the authentication code and the actual method implementation often end up needing to access the same data from the database; this would have to come with some way to also pass data between the two, and at that point it doesn't seem worth it. Also, you might often need to load doc A, do an auth check, then load doc B, do another auth check, etc.

So to me it looks like such an API would work for simple cases but not complex ones, maybe that is good enough?

[johanbrook]

Ah, gotcha. I see how you mean. I don't think it's a good idea exploring how to pass data between the method body and an auth hooks – it feels out of scope.

Or, could this API's use case be trivial user auth scenarios only? Perhaps only for checking the existence of this.userId, role checking, etc?

But then again, it's only about moving code from one place to another:

new ValidatedMethod({
   auth() {
      // .. → to here
   },
   run() {
     // From here..
   }
})

Are there any other upsides, besides cleaner function bodies and separation of concerns?

[stubailo]

The only thing I could think of that would be a real benefit would be running the auth code without running the body, which is what we can easily do with validation. For example:

const method = new ValidatedMethod({
  authorize(...) { ... }
});

// Somewhere else, for example, our UI:
if (method.authorize(Meteor.userId())) {
  // Allowed to do the action
}

You could use this to show/hide certain buttons by looking at the auth logic of the Method, etc.

I think the best way to do this would be via Method mixins. So for example, let's say you wanted to do auth with roles:

{
  name: 'xxx',
  mixins: [MethodRolesMixin],
  validate: ....
  roles: ["admin"],
  run() {
   // .. the roles mixin checks the roles for us!
  }
}

Basically, the mixin could wrap the run method for us, and look at the roles property.

Perhaps I should go code up that mixin functionality, since it would help with a lot of other issues as well.

[stubailo]

In fact, the mixin function could get rid of the validate built-in as well, letting people replace it with other types of functionality.

[johanbrook]

Yep, I too think mixin functionality is most future proof and open for extensions like this :)

Would each element in a mixin array be a function (or class) which would receive some props, or the method invocation?

[stubailo]

I think it would get access to the Method definition, and then it would be able to access any special properties (like roles) and then wrap the run method as it sees fit, or create new methods. Basically, it should be as flexible as possible so that package authors don't have to resort to monkey-patching, or wrapper functions or something.

[johanbrook]

Sounds ace! :ok_hand: