Bug (CSRF undefined) + Fix #509

Closed
jorisroling opened this Issue Mar 12, 2012 · 3 comments

Projects

None yet

4 participants

@jorisroling

I came across an error _"Cannot read property 'csrf' of undefined"
(connect/lib/middleware/csrf.js line 79)

This was due to me using the session.ignore array for certain paths.

To make connect tolerant for that specific situation I present the following fix.

In file connect/lib/middleware/csrf.js

module.exports = function csrf(options) {
  var options = options || {}
    , value = options.value || defaultValue;

  return function(req, res, next){

    if (req.session) { // <<<<<<<<<<<<<<< This condition should fix it
        // generate CSRF token
        var token = req.session._csrf || (req.session._csrf = utils.uid(24));

        // ignore GET (for now)
        if ('GET' == req.method) return next();

        // determine value
        var val = value(req);

        // check
        if (val != token) return utils.forbidden(res);
    }
    next();
  }
};

Man There is no need to do this

Just set csrf() in the follow order and will work!

app.configure(function(){
    app.use(express.bodyParser());
    app.use(express.methodOverride());
    app.use(express.cookieParser('treinosmart'));
    app.use(express.session({key: 'ts', cookie: {maxAge: 60000}}));
    app.use(express.csrf()); // It must stay after the express.session() and express.cookieParset().
});

This will work on Connect and Express, which I showed that example

Iftahh commented Oct 28, 2012

I agree with @jorisroling, I bumped into it as well.
csrf middleware isn't working if you have urls that skip the session middleware (by using the session.ignore array).

I see the ignore array was removed a while ago by 73bf054

but when it was included, it was incompatible with csrf middleware.

@caio-ribeiro-pereira your example does not trigger the bug because you have no ignore urls set up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment