How to test a Meteor.method that internally calls another Meteor.method when both require `this.userId`?

If two methods both check that it contains this.userId before proceeding, and say Method A calls Method B internally, how can you pass the this.userId in a test that runs mockMethodCall(MethodA, {context: {userId}})?

For e.g.,

Meteor.methods({
    'MethodA': function() {
       if(!this.userId) {
             console.log('Not authorized!');
             return;
       }
       const some_id = //... <insert someData into collection1> and return some_id
       Meteor.call('MethodB', some_id);
    },
 
    'MethodB': function(some_id) {
       if(!this.userId) {
             console.log('Not authorized!');
             return;
       }
       // <insert someData into collection2>
    }
})

Then in a test file:

    const mockMethodCall = require('meteor/quave:testing').mockMethodCall;
     ...
    it('inserts new document in two collections', function() {
          mockMethodCall('MethodA', {context: {userId: 'user_id123'}}) 
          // ^ this will throw an error ('Not authorized') because the 'userId' is not passed to 'MethodB'
    });

How can I pass userId to ‘MethodB’ in the test?

Thanks!

I would always separate the Meteor method from the actual logic in that method and make userId a parameter. Makes testing so much easier.

Example:

export const mymethoda = userId => {...}
import { mymethoda } from '/path/to/...'

Meteor.methods({
  methodA() {
    return mymethoda (this.userId)
  }
})
2 Likes

But wouldn’t this compromise security?

From what I understand, methods are called on both client and server. If you have a method that takes a userId as paramter, a malicious actor could (on the client) manually call mymethoda and pass someone else’s userId (assuming they already have it, or perhaps they might try brute force etc…)

It would not as long as the internal function mymethoda is only called from the Meteor.method as shown in the example above. Apart from that this function should always only be accessible on the server and nothing esle should access ist besides using the Meteor method.

Exception: importing it when running your tests :slight_smile:

I see Methods as equivalent to HTTP API endpoints, and in a restful app, I just like I wouldn’t have the server make HTTP calls to itself, I don’t have Meteor servers make method calls to itself either.

Much eaiser if you have the method perform the authorization and then either throw or pass the userId and any other context to an internal server function (ie. service) that then performs the required action.

BUT, this is all highly opinionated, other ways of designing your app aren’t wrong, but this way does make it easier to test!

Sorry, I’m still a bit confused. From the docs it states:

"…If we defined this Method in client and server code, as all Methods should be, a Method simulation is executed in the client that called it.

So following @jkuester example,

export const mymethoda = userId => {...}
import { mymethoda } from '/path/to/...'

Meteor.methods({
  methodA() {
    return mymethoda (this.userId)
  }
})

If my frontend makes a call to methodA, it is executed on the client first, which means the client method methodA needs access to mymethoda (or it will throw an error saying "mymethoda is not a function…"), and if the client has access to methoda, then a person can manually call methoda and manually pass in a userId, which would compromise security, right? Or am I missing something fundamental here.

The fundamental you’re missing is that the client only performs a simulation, the server is the source of truth. If the client does something funny that it’s not allowed to do and tries to trick the simulation, that won’t actually be reflected on the server and their invalid changes will be rolled back as soon as the client gets the update from the server.

For example:

User A is trying to delete resource Foo, but they don’t have permisisons to do so.
They call the method (which runs on both client and server).
Simulation throws permission error and does nothing
Server throws permission error and does nothing.

Now, User A has tricked the client simulation into thinking they have permission to delete it:

They call the method (which runs on both client and server).
Simulation completes and shows resource deleted
Client UI updates to reflect changed data
Server throws permission error and does nothing.
Client sees error and rollsback changes done in simulation
Client UI updates to reflect state on server


That said, it is worth mentioning that Methods should always use this.userId and not use a passed argument. My suggestion is to check permissions in the original method and then pass the context from this.userId to another function which is not a Meteor Method

2 Likes

Oh ok, that makes sense, thanks for clearing that up!

Also I just realized, if a malicious actor tries to manually call mymethoda manually passing in some userId, it is run only on the client, and it means nothing because the client would only have data associated to his/her account in their minimongo db, so another accounts userId wouldn’t even exist in the their minimongo, right?

1 Like

Yes, that’s right

Which is a good reminder why permissions checks on publications are important

1 Like

Testing methods with Meteor, just don’t use this.userId at all. Instead use Meteor.userId() in your methods to retrieve the user id.

That enables you to replace the Meteor.userId() function with whatever you want it to return during testing, and it is set globally. It also enables you to test security and rights when calling methods. Example:

Meteor.userId = () => null to simulate a logged out user
Meteor.userId = () => 'someAdminId' to simulate an admin
Meteor.userId = () => 'someUserId' to simulate a user
etc

1 Like