18

I have set up a web application with some internal pages requiring a login. I used Node with Express.js to set up the server and controlling the routes and authentication works fine.

I came up to a @zanko suggestion in a question related to the same application to avoid the replication of the authentication code in the route of every page, like is now.

At the moment my app.js looks like this (the following is an excerpt):

var session = require('express-session');
//use sessions for tracking logins
app.use(session({
  secret: 'mercuia',
  resave: true,
  saveUninitialized: false,
  store: new MongoStore({
  mongooseConnection: db
})
}));

// serve static files from template
app.use(express.static(__dirname + '/public'));

// include routes
var routes = require('./routes/router');
app.use('/', routes);

// catch 404 and forward to error handler
app.use(function (req, res, next) {
  var err = new Error('File Not Found');
  err.status = 404;
  next(err);
});

// error handler
// define as the last app.use callback
app.use(function (err, req, res, next) {
  res.status(err.status || 500);
  res.send(err.message);
});

and my authentication method (in routes.js) looks like this (in the example, for the route /clientPage):

// GET route after registering
router.get('/clientPage', function (req, res, next) {
  User.findById(req.session.userId)
    .exec(function (error, user) {
      if (error) {
        return next(error);
      } else {      
        if (user === null) {     
          var err = new Error('Not authorized! Go back!');
          err.status = 400;
          return next(err);
        } else {
          return res.sendFile(path.join(__dirname + '/../views/clientPage.html'));
        }
      }
    });
});

How can I write an authentication middleware instead (using the same logic) and call it for all and only the requiring routes?

user1944491
  • 559
  • 1
  • 8
  • 24
Alex
  • 203
  • 1
  • 2
  • 8
  • 2
    I am wondering how you populate your `req.session.userId` I cant see any middle ware in `app.js` that does this – Zanko Nov 27 '17 at 17:13

2 Answers2

30

You can create a new module called auth.js and then use it to check if users are authorized or not:

auth.js

module.exports.isAuthorized  = function(req, res, next) {

    User.findById(req.session.userId).exec(function (error, user) {
        if (error) {
            return next(error);
        } else {      
            if (user === null) {     
                var err = new Error('Not authorized! Go back!');
                err.status = 401;
                return next(err);
            } else {
                return next();
            }
        }
    });
}

routes.js

var auth = require('./auth');

// GET route after registering
router.get('/clientPage', auth.isAuthorized, function (req, res, next) {
    res.sendFile(path.join(__dirname + '/../views/clientPage.html'));
});
YouneL
  • 8,152
  • 2
  • 28
  • 50
  • Should your `require('auth');` be relative part? That will look inside `node_modules` folder – Zanko Nov 27 '17 at 17:16
  • 1
    I just gave him the idea to deal with the problem, also in your answer `app.use(restrictMiddleware);` apply restrict middleware to all routes, – YouneL Nov 27 '17 at 17:22
  • Yea it will apply to all route underneath the app.use(restrictMiddleware) but not anything before that. Yours is more flexible but might need to put it on alot of routes depending on how many routes he wants to restrict – Zanko Nov 27 '17 at 17:25
  • I agree, but you can just hoist the function into the route before the middleware. Personally I prefer separating them (better mental pattern for me) because we can clearly separate the unauthorized route from authorized one instead of mixing them together. But its the design decision OP has to make, there is no silver bullet for any solution. – Zanko Nov 27 '17 at 17:54
  • 4
    Really like this answer. I would change the err.status to 401 though since it's an authorization issue. – petithomme Feb 28 '19 at 17:06
  • This answer effectively prevents any access to the page because it does NOT cause the browser to prompt for the authorization. Instead of an error, return this: `res.writeHead(401, { 'WWW-Authenticate': 'Basic realm="Basic Authentication"' }); res.end('Authorization is needed');` – user1944491 Oct 02 '19 at 16:33
13

Create a module (a file that exports a function, in this case a middleware function). A middleware function has following signature function (req, res, next) { .... }

restrict.js

module.exports = function (req, res, next) {
  User.findById(req.session.userId)
    .exec(function (error, user) {
      if (error) {
        return next(error);
      } else {
        if (user === null) {
          const err = new Error("Not authorized! Go back!");
          err.status = 400;
          return next(err); // This will be caught by error handler
        } else {
          return next(); // No error proceed to next middleware
        }
      }
    });
};

app.js

// serve static files from template
app.use(express.static(__dirname + '/public'));

// include routes
const routes = require('./routes/router');

//If you have a more granular route you can split it 
const someOtherRoute = require('./routes/someotherRoute');

const restrictMiddleware = require("./restrict");

app.use("/", someOtherRoute); // this route will not be check for authorization
app.use(restrictMiddleware);
app.use('/', routes);

// catch 404 and forward to error handler
app.use(function (req, res, next) {
  const err = new Error('File Not Found');
  err.status = 404;
  next(err);
});

// error handler
// define as the last app.use callback
app.use(function (err, req, res, next) {
  res.status(err.status || 500);
  res.send(err.message);
});

I would use const and let if your environment support it. Its 2017 :)

Zanko
  • 4,298
  • 4
  • 31
  • 54