New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: Enable this.setUserId in server-initiated methods #6388

Closed
turbobuilt opened this Issue Mar 3, 2016 · 8 comments

Comments

Projects
None yet
4 participants
@turbobuilt
Copy link

turbobuilt commented Mar 3, 2016

Hello,

I tried running this.setUserId in a server initiated method, and I was informed that such behavior was not allowed on a server initiate method. I'm not sure as to why this is the case - and I'm not sure of how difficult it would be to allow this. But the need for it in my application is acute, and I can't imagine that I'm the only one in this situation.

Here's the use case:
I'm making a REST api, and need to execute some methods as though I were a logged in user when these were called. Of course, you may say that I should just use a server-side auth package that checks the hash. That works if I'm rolling everything from scratch. However, if I'm using third party functions that depend on Meteor.userId() working, I'm out of luck. One such important package is the collection-hooks package. I use collection hooks to manage all my authentication to and from the database. It depends on Meteor.userId() working. If it doesn't database transactions are always denied.

One reason I can imagine why you wouldn't have this functionality is because the userId is associated with a specific connection. If there isn't a connection - you can't associate a userId with it. However, there has got to be a way that you can keep track of the userId in the current fiber, even on server initiated method calls.

Please let me know if this is doable. I'd love to see this integrated into the meteor core. As you can see what I have written, my authentication mechanism will have to be completely rewritten if I can't find a way to do this, or some workaround. If you decide not to implement it, do you have a workaround?

Thank you!

@stubailo stubailo self-assigned this Mar 8, 2016

@stubailo

This comment has been minimized.

Copy link
Contributor

stubailo commented Mar 8, 2016

I would suggest that this is a flaw in the collection hooks package. I think we should move away from having a magical global variable on the server that changes when switching fibers, because it makes life hard when integrating with non-Meteor libraries.

But you can easily work around this by creating a new current method invocation on the server, like so:

// put whatever you want in these properties
var invocation = new DDPCommon.MethodInvocation({
  isSimulation: false,
  userId: "myUserId",
  setUserId: setUserId,
  unblock: unblock,
  connection: self.connectionHandle,
  randomSeed: randomSeed
});

DDP._CurrentInvocation.withValue(invocation, () => {
  Meteor.userId(); // "myUserId"
});

This is basically the code that runs when a method is called from the client. Let me know if this doesn't work for you, but I'm pretty sure the next Meteor data system (Apollo) won't have global state on the server like this, instead passing through the data to places that need it explicitly.

@turbobuilt

This comment has been minimized.

Copy link

turbobuilt commented Mar 9, 2016

Hi Stubailo,

Thank you very much for your input. I tried writing the code you suggested, and get the error "Error: ReferenceError: DDPCommon is not defined". Do you know why it's undefined? This is in the Server folder.

@stubailo

This comment has been minimized.

Copy link
Contributor

stubailo commented Mar 9, 2016

Ah, that object is not directly exposed to apps, you have to ask for it:

const DDPCommon = Package['ddp-common'].DDPCommon;

... rest of the code here
@turbobuilt

This comment has been minimized.

Copy link

turbobuilt commented Mar 9, 2016

Stubailo, you are a hero!

I spent a long time looking for how to do this. I browsed around through the source for hours looking for something, anything to do this. Figuring this out would have taken me weeks, if I could do it at all without your help. Thank you very much.

This was my test code that got it working. I have 2 more questions: does randomSeed matter? Do I need to make it a cryptographically secure number? Also, is it ok for me to do {} for the connection? It didn't throw any errors, and worked fine, but I wonder if that's insecure in any way.

        var invocation = new DDPCommon.MethodInvocation({
          isSimulation: false,
          userId: "dbAnEgsK22x5NAghP",
          setUserId: ()=>{},
          unblock: ()=>{},
          connection: {},
          randomSeed: Math.random()
        });

        DDP._CurrentInvocation.withValue(invocation, () => {
          Meteor.userId(); // "myUserId"
          MyCollection.insert({_id:"testuser"})
        });
@stubailo

This comment has been minimized.

Copy link
Contributor

stubailo commented Mar 9, 2016

does randomSeed matter?

Random seed is used to make sure that client and server inserts generate the same ID. So if you are not running any code on the client for optimistic UI, it isn't super important. If you want your collection IDs to be cryptographically secure, then you need a secure seed, but if you don't then just Random.id() will do fine!

is it ok for me to do {} for the connection?

I think it only matters if in your method implementation you use that property. So if your method never calls this.connection then you're good I think.

@turbobuilt

This comment has been minimized.

Copy link

turbobuilt commented Mar 10, 2016

Ok, thanks for the information stubailo!

@dolle39

This comment has been minimized.

Copy link

dolle39 commented Jan 1, 2018

I have tested this solution but it does not fully work when the method in turn initiates an asynchronous callback. So for example, in the method I start a child process and when finished I want to handle the results. After the process has exited and the callback is fired, it seems that the entire invocation context is gone. Andy ideas how to handle that case?

Example:

startProcessAndHandleResults = function() {

const process = ChildProcess.spawn(this.getCommand().executable, this.getCommand().args);
let someObjectINeedLater = ....
process.on('exit', Meteor.bindEnvironment(function (code, signal) {
    let reply = {};
    reply.id = let someObjectINeedLater.id;   // <----- THIS DOES NOT EXIST ANYMORE
    ....

}

And the invocation:


        let invocation = new DDPCommon.MethodInvocation({
          isSimulation: false,
          userId: myUserId,
          setUserId: ()=>{},
          unblock: ()=>{},
          connection: {},
          randomSeed: Math.random(),
        });

        DDP._CurrentInvocation.withValue(invocation, function () {
          startProcessAndHandleResults();
        });

When the child process exits someObjectINeedLater is undefined.

@mitar

This comment has been minimized.

Copy link
Collaborator

mitar commented Jan 1, 2018

reply.id = let someObjectINeedLater.id; is wrong, you probably want:

reply.id = someObjectINeedLater.id;
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment