_req_user is undefined on pre/post remove hooks

[poksme]

The following middlewares output an undefined _req_user :

Article.schema.pre('remove', function reqUserDebug (next) {
    console.log('================ pre remove _req_user ================')
    console.log(this._req_user);
    return next();
});

Article.schema.post('remove', function reqUserDebug (next) {
    console.log('================ post remove _req_user ================')
    console.log(this._req_user);
    return next();
});

output:

================ pre remove hook _req_user ================
undefined
================ post remove hook _req_user ================
undefined

I do not know if it is useful but here is some background about my project regarding this issue:

I am developing a website with some role limitation on the back-office. I am using the _req_user on pre save hook to limit modifications. But without the _req_user on pre remove hook it is impossible to limit the remove rights.

[snowkeeper]

The admin ui adds _req_user with updateHandler (https://github.com/keystonejs/keystone/blob/master/lib/updateHandler.js#L174-L175).

@JedWatson are you interested in supporting _req_user for remove also? Would adding a remove method to updateHandler be ok?

UpdateHandler.prototype.remove = function(callback) {

    if ('function' !== typeof callback) {
        callback = function() {};
    }

    // Make current user available to pre/post remove events
    this.item._req_user = this.user;

    this.item.remove(callback);

};

Our deletes can be something like

req.list.model.findById(req.query['delete']).exec(function (err, item) { 
    if (err || !item) return res.redirect('/keystone/' + req.list.path);

    item.getUpdateHandler(req).remove(function (err) {
        if (err) {
            console.log('Error deleting ' + req.list.singular);
            console.log(err);
            req.flash('error', 'Error deleting the ' + req.list.singular + ': ' + err.message);
        } else {
            req.flash('success', req.list.singular + ' deleted successfully.');
        }
        return res.redirect('/keystone/' + req.list.path);
    });
});

[r1b]

One workaround I have been using is to turn on tracking for the target model & use the updatedBy field to determine the user who made the changes.

[poksme]

I could not get the updatedBy field to work properly but I found a solution, I tried to keep it as clean as possible without modifying any keystone source file.

There may be some extra code because my project uses FlowType annotations.

1 - You will need to install a couple of npm packages

npm install --save continuation-local-storage cls-mongoose

2 - At the top of your keystone.js file add the following code

var cls                 = require('continuation-local-storage');
var mongoose            = require('mongoose');
var clsMongoose         = require('cls-mongoose');

var ns = cls.createNamespace('my session');

clsMongoose(ns);

keystone.init({
[...]
    'mongoose': mongoose,
[...]
});

keystone.start();

This should shimmer-wrap mongoose to make it safe to use with continuation local storage and then you pass it to keystone init so it uses this patched version of mongoose.

3 - Create two new middlewares in your middleware.js

var cls = require('continuation-local-storage');

[...]

exports.initContinuation = function (req :HTTPRequest, res :HTTPResponse, next :ExpressCallback) {
    var ns = cls.getNamespace('my session');
    ns.bindEmitter(req);
    ns.bindEmitter(res);

    ns.run(function () {
        // This extra function level is needed. Calling ns.run(next); directly will fail
        next();
    });
}

exports.continuationUser = function (req :HTTPRequest, res :HTTPResponse, next :ExpressCallback) {
    var ns = cls.getNamespace('my session');

    // At this point you could save anything from req.
    ns.set('user', req.user);

    next();
}

The first middleware will patch your Express queue in order to be continuation local storage compliant.
The second middleware is straightforward, it saves the req.user in your continuation local storage namespace.

4 - Add your middlewares to pre routes hook in index.js

keystone.pre('routes',
    middleware.initContinuation,
    middleware.continuationUser
);

Be careful to add the middlewares in the right order and on pre:routes hook.

5 - Access your save req.user on any mongoose hook in your MyModel.js

var cls = require('continuation-local-storage');

[...]

MyModel.schema.pre('remove', function debugSessionUser (next) {
    var session = cls.getNamespace('my session');

    console.log(session.get('user'));
    return next();
})

You can now check your console to see if you have access to the current user.

This worked for me but any feed back is welcomed 😃

[poksme]

As I sent my previous answer I was thinking I might have just re-wrote keystone.setand keystone.get.

So I just tried to code a simple middleware that does

exports.saveUser = function (req :HTTPRequest, res :HTTPResponse, next :ExpressCallback) {
    keystone.set('user', req.user);
    next();
}

and I could retrieve the user from the remove hook on MyModel.js

MyModel.schema.pre('remove', function debugSessionUser (next) {
    console.log(keystone.get('user'));
    return next();
})

It took me a lot of tinkering around to finally use one of the most obvious keystone built-in...

[poksme]

The question now is:
Does keystone.get and set work well with concurrency? Are the variables saved using these methods common to the whole app or common to the user session? I believe they are common to the whole app since keystone.set is generally used for app options.

In this case the continuation local storage might be safer.

[r1b]

If you use update scripts, leveraging the updatedBy field will not work (it is unset in the context of an update script). I am now using @poksme 's solution.

[wmertens]

I'm sure the .set/.get solution won't handle concurrency.

I'm proposing a session context object (fill like you wish) that will be passed to all hooks in #3187. I'll close this issue in its favor, please discuss requirements there.

[Nefil33]

Was there actually a solution for this? Sorry just looked around and all issues are closed but I still have this issue.