Stopping the middleware chain in express4

1
votes

Below is the code which has a middleware function "checkLogin" added. The eheckLogin middleware should redirect to login page if the user is not logged in. If the user is logged in it should render the requested page. 

var express = require('express');
var router = express.Router();
var mongoUri = 'mongodb://localhost/mydb';
var MongoClient = require('mongodb').MongoClient;

function mongoDBConnect(req, res, next) {
    MongoClient.connect('mongodb://localhost/gadda_db', function(err, db) {
    if(!err) {
        req.db = db;
        next();
    } else {
        res.send('unable to connect to mongodb: err = ' + err);
    }
    });
};

function checkLogin(req, res, next) {
    if (req.loggedin) {
    next();
    return;
    }

    if (req.loggedin && req.url === '/login') {
    res.redirect('http://' + 'localhost:3000' + '/game/my_game'); 
    return;
    }

    if (!req.loggedin && req.url === '/login') {
    next();
    return;
    }
    req.db.collection('game_users', function (err, collection) {
        if (err) {
        res.send("error while reading game_users: err " + err);
    }
        collection.findOne({user: req.cookies.user, password: req.cookies.password},
               function (err, user) {
                   if (err) {
                   res.send("error here");
                   return;
                   }
                   req.loggedin = true;
                   next();
                   return;
               });
    });

    res.redirect('login');
    return;
};

router.use(mongoDBConnect);
router.use(checkLogin);

router.get('/', function(req, res) {
    res.redirect('http://' + 'localhost:3000' + '/game/my_game');
});

router.get('/login', function(req, res) {
    res.render('gadda_login', {title: 'gadda', error: ''});
});

When the request to localhost:3000/gadda is sent the following error message is printed on the server

GET /gadda/ 302 34ms - 66b Error: Can't set headers after they are sent. at ServerResponse.OutgoingMessage.setHeader (http.js:691:11) at ServerResponse.res.set.res.header (/home/hhk/src/nodejs_projects/gadda_v2/node_modules/express/lib/response.js:551:10) at ServerResponse.res.send (/home/hhk/src/nodejs_projects/gadda_v2/node_modules/express/lib/response.js:132:12) at fn (/home/hhk/src/nodejs_projects/gadda_v2/node_modules/express/lib/response.js:778:10) at View.exports.renderFile [as engine] (/home/hhk/src/nodejs_projects/gadda_v2/node_modules/ejs/lib/ejs.js:318:3) at View.render (/home/hhk/src/nodejs_projects/gadda_v2/node_modules/express/lib/view.js:76:8) at Function.app.render (/home/hhk/src/nodejs_projects/gadda_v2/node_modules/express/lib/application.js:519:10) at ServerResponse.res.render (/home/hhk/src/nodejs_projects/gadda_v2/node_modules/express/lib/response.js:782:7) at Layer.module.exports [as handle] (/home/hhk/src/nodejs_projects/gadda_v2/app.js:54:9) at trim_prefix (/home/hhk/src/nodejs_projects/gadda_v2/node_modules/express/lib/router/index.js:252:17) router.get /login GET /gadda/login 200 11ms - 546b GET /stylesheets/style.css 200 4ms - 110b

1
node.jsexpress
Does your login form POST to the same URL (/login)? If so you're logic maybe skipping the check to see if they are logged in (within the database call) by matching on the prior if statement (the req.url === '/login'). Also, you're redirecting to login (res.redirect('login');) prior to allowing your database callback to fire. – dylants
@dylants: The post is done to '/users/new' url. router.post('/user/new' , function(req, res) {....}). The problem seems to be that after a res.redirect('login') in checkLogin, again a req.redirect(...) is exectued in the router.get('/', fun(...)). The lesson learned is it is not possible to avoid the execution of the function in router.get('/', function()..{}), no matter what ever is done in the middleware. Although this might be intuitive to many, but it is very new to me. I was looking the router.get fns as a part of the middlware chain, expected to break it from some middleware function. – Talespin_Kit
It is possible to avoid the route call via middleware. I think it might have to do with the way the checkLogin function is written. I've tried to explain in the answer below, please let me know if that makes sense. – dylants

1 Answers

3
votes

It appears from the code you pasted above that you are redirecting to "login" prior to waiting for the database find to return. Try refactoring your checkLogin function to the following:

function checkLogin(req, res, next) {
    if (req.loggedin) {
        if (req.url === '/login') {
            res.redirect('http://' + 'localhost:3000' + '/game/my_game');
            return;
        }
        next();
        return;
    }

    if (!req.loggedin && req.url === '/login') {
        next();
        return;
    }

    req.db.collection('game_users', function(err, collection) {
        if (err) {
            res.send("error while reading game_users: err " + err);
            return;
        }

        collection.findOne({
            user: req.cookies.user,
            password: req.cookies.password
        }, function(err, user) {
            if (err) {
                res.send("error here");
                return;
            }

            if (user) {
                req.loggedin = true;
                next();
                return;
            } else {
                res.redirect('/login');
                return;
            }
        });
    });
}

I've moved the redirect to /login to only happen when the database call comes back without a matching user. This means that you've searched but not found any match, meaning that the supplied user and password (contained within the cookies?) is not correct.

The point here is to only perform a res.send or res.redirect once, and if we should not do this in the middleware, then simply call next(). By not calling next() you're code won't hit the route code.

Be careful here though, since this code won't allow any requests to hit their destination unless it's an authenticated request. This includes things like the favicon or assets.