getCurrentContext.get(<property>) sometimes returns other request/user's information

[benjaminhaven]

I'm running into what looks like a concurrency issue using the getCurrentContext in operation hooks.

We are using the context to store the user's access token, userId and groupId. This is done in middleware which is set on the auth:after.

getUserGroups(req.accessToken.userId),
      (user, groups) => {
        if (!user) {
          return next(new Error('No user with this access token was found.'));
        }
        loopbackContext.set('currentUser', user);
        loopbackContext.set('currentUserGroups', groups);

We attempt to get these values as so:

  getCurrentUser() {
    const ctx = this.app.loopback.getCurrentContext();
    const currentUser = ctx && ctx.get('currentUser') || null;

    return currentUser;
  }

This getCurrentUser method is used within a operation hook

Model.observe('access', (ctx, next) => {

      logger.debug('ACCESS observer - Observing access for', modelName);
      const currentUser = this.getCurrentUser();

The problem I am experiencing is that when I have several users logged into the system at the same time on a production environment (4 workers) occasionally the this.getCurrentUser() call will return with the WRONG user. This leads to some major security issues.

I'm trying to figure out if this is something I'm doing incorrectly or if this is just a known issue with the context / CLS. I've read up on some related issues people have posted such as #2094 and #1495.

[benjaminhaven]

Sorry for closing / reopening - wasn't sure if anyone was monitoring this

[benjaminhaven]

Just to let you know, it seems like the issue w. the context is not specific to the remote hooks but actually related to trying to get the context from within a promise chain. The reason we saw it in the hooks was because those were being invoked by Model.create / Model.find operations that were within a promise chain. The context outside of the chain is correct, when we go into a promise chain it seems to pick up a previous 'state'. That seems to narrow it down a fair bit.

Also we notice that the CLS object has a _set property which seems to preserver different context's as a stack.

Just wanted to pass this along in case it wasn't something you had seen. From reading the issues on the CLS github it seems promises cause problems without workarounds / shims people have created.

[bajtos]

I am afraid Node.js does not provide any robust solution for continuation-local-storage at this moment, therefore we cannot provide a reliable getCurrentContext functionality :( See https://github.com/strongloop/loopback-context#warning and https://github.com/strongloop/loopback-context#known-issues for more details.

Promises are one of the known cases that don't work. However, IIRC there are ways how to monkey patch them to support continuation local storage. What promise library are you using?

I am closing this issue as there isn't much we can do about it here in loopback core.

Please open a new issue in https://github.com/strongloop/loopback-context/issues/new and provide us with a small app reproducing the problem, see http://loopback.io/doc/en/contrib/Reporting-issues.html#bug-report. See also the discussion in strongloop/loopback-context#17 which may provide relevant information for your use case too.