ExpressJs Passportjs de-serializes user object for every request to a route
Asked Answered
S

2

3

I have a ExpressJS app that is using Passportjs to authenticate with Facebook and everything is working as expected exception for one issue.

I have vehicle.js under /routes/ which contains some routes (router.gets and router.posts) that need authentication and some that don't. If user is logged in then every request handled by vehicle.js causes User de-serialization which is a Mongoose lookup. How can I avoid these unnecessary Mongoose lookups when request is made to a router.get and/or router.post that do not need authentication?

I have already looked up this SO question and it does not address my problem (I have declared static resources above passport, so they are not authenticated).

Passport configs in app.js are shown below:

// Configuring Passport
var passport = require('passport');
var expressSession = require('express-session');

app.use(expressSession({secret: 'thisIsSecret'}));
app.use(passport.initialize());
app.use(passport.session());

// Using the flash middleware provided by connect-flash to store messages in session
// and displaying in templates
var flash = require('connect-flash');
app.use(flash());

// Initialize Passport
var initPassport = require('./passport/init');
initPassport(passport);

//passing passport object could be the reason why all requested that are 
//mapped in vehicle cause user de-serialization. Not sure if there is any
//alternative approach than following line that passes passport??
var vehicle = require('./routes/vehicle')(passport);

The following isAuthenticated is in vehicle.js

var isAuthenticated = function (req, res, next) {
  if (req.isAuthenticated())
    return next();
    // if the user is not authenticated then redirect him to the login page
    res.redirect('/vehicle/api/login');
}

Followed by a series of routes that handle logging in, logging out, as well as some actions on vehicle.

module.exports = function(passport) {
  router.get('/api/login', function(req, res) {
    res.render('vehicle/login', { message: req.flash('message') });
  });
  router.post('/api/login', passport.authenticate('login', {
    successRedirect: '/vehicle/api/list/mine',
    failureRedirect: '/vehicle/api/list',
    failureFlash : true
  }));
  ...
  ...
  ...
  router.post('/api/upload', isAuthenticated, function(req, res) {
    //this route needs to  be authenticated, so it works fine, 
    //deserialization done, mongoose lookup, no problem
  }); 
  router.get('/api/image/:vehicleId/:filename', function(req,res) {
    //this route does not need authentication, but causes User 
    //de-serialization and Mongoose lookup
  }); 
 return router;
}

Is it because of the following line that every request to vehicle.js causes User de-serialization when a user is logged in?

var vehicle = require('./routes/vehicle')(passport);

One way to avoid such unnecessary de-serialization would be to separate routes that do not need authentication from vehicle.js to a different file and do not pass that passport object to that file (as it is passed to vehicle.js in app.js). I don't know if that is the correct way to resolve this issue.

Subjunction answered 14/12, 2015 at 22:24 Comment(0)
S
2

You can wrap the passport middleware inside a custom middleware that only invokes it for your specified routes. So Instead of:

app.use(passport.session());

you could:

app.use(function(req, res, next){
  if(req.url.match('api/image'))
    next(); // do not invoke passport
  else
    passport.session()(req, res, next)
    // same as doing == app.use(passport.session())
});
Screech answered 15/12, 2015 at 1:9 Comment(4)
Exactly what I was looking for, thank you. I have a question though, if not mistaken the above if condition is executed for every request. Right? Not sure what is the cost of that if condition. Also, do you think it is efficient to use if condition or apply passport.session() to specific route (after separating secure/non-secure routes)?Subjunction
@Subjunction cost of adding a middleware is practically nothing. I'm measuring ~0.06ms. It'd probably be less in production. So to me, efficiency-wise it wouldn't matter much. I would probably base the choice on other factors, like whatever code makes more sense to reason about.Screech
@Subjunction In addition to what laggingreflex mentioned, I also tweaked the passport deserialize function to store a temporary (like 10-20 seconds) in-memory copy of the "deserialized" user value. Reason is many a times the user navigates through pages quickly or a page might have multiple requests made. In such cases, I lookup mongodb once and for the next 20 seconds or so any additional requests are handled from in-memory rather hitting mongodb/db-server. This made my webpage responses quite a bit faster.Lotta
@Lotta is there any way I can have a look at the tweaked version of the passport's deserialized function? You could add it as an answer, I would definitely give you an upvote.Subjunction
B
0

If you use passport.session() middleware, deserialize will happen for every route: https://github.com/jaredhanson/passport/blob/33075756a626999c6e2efc872b055e45ae434053/lib/strategies/session.js#L53-L69

The solution would be to add it only to ones which use passport.

Brezhnev answered 14/12, 2015 at 22:56 Comment(1)
What do you mean by The solution would be to add it only to ones which use passport?Subjunction

© 2022 - 2024 — McMap. All rights reserved.