Modularize this node.js express server code for routes and session handlers?

Question

I have express node.js server code using Passport. It receives a user to authenticate it and send him to some data registration form.

Some concerns:

• My whole routes definition depends upon a MongoDB connection using mongo-db ,

• but model used by Passport is done through another connection by mongoose.

• The main problem is that even though Passport it's doing it's work, I still can go to localhost/registro directly no matter I didn't logged in first.

• Code already looks a little messy and disorganized.

I care about security. I'd also like to have some information about the user during the session time, but am unsure how to best achieve that.

This is my server.js:

var express = require('express')
var mongodb = require('mongodb')
var mongoose = require('mongoose')
var bodyParser = require('body-parser')
var passport = require('passport')
var LocalStrategy = require('passport-local').Strategy;
var app = express()
var BSON = mongodb.BSONPure
app.use(passport.initialize());
app.use(passport.session());
app.use(express.static(__dirname+"/public"))
app.use(bodyParser())
var MongoDBClient = mongodb.MongoClient
mongoose.connect('mongodb://localhost/psicologosTuxtepecDB')
var Schema = mongoose.Schema
var userCredential = new Schema({
 username: String,
 password: String
}, {
 collection: 'members'
})
var userCredentials = mongoose.model('members', userCredential)
passport.serializeUser(function(user, done) {
 done(null, user);
})
passport.deserializeUser(function(user, done) {
 done(null, user);
})
passport.use(new LocalStrategy(function(username, password, done) {
 process.nextTick(function() {
 userCredentials.findOne({
 'username': username, 
 }, function(err, user) {
 if (err) {
 return done(err);
 }
 if (!user) {
 return done(null, false);
 }
 if (user.password != password) {
 return done(null, false);
 }
 return done(null, user);
 });
 });
}));
MongoDBClient.connect("mongodb://localhost/psicologosTuxtepecDB", function (error, psicologosTuxtepecDB) {
 if (error) {
 console.log("We've got a connection error, so far we should take this function better for a correct debug")
 }
 else {
 console.log("Connection to psicologosTuxtepecDB has been successful")
 // Seleccionamos una colección
 var psicologosCollection = psicologosTuxtepecDB.collection("psicologos")
 app.get('/registro', function(request,response) {
 response.sendfile("public/html/registro.html")
 })
 // Cuando nos hagan una petición HTTP de tipo POST en la ruta psicologos...
 app.post("/psychos", function(request, response) {
 var psychologist = {
 personalData: request.body._personalData,
 professionalData: request.body._professionalData,
 professionalInterests: request.body._professionalInterests
 }
 psicologosCollection.insert(psychologist, function(error, responseFromDB) {
 if (error) {response.send(responseFromDB)}
 console.log("Se ha insertado: "+ JSON.strinfigy(responseFromDB))
 response.send(responseFromDB)
 })
 })
 app.get("/psychos/:id", function(request, response) {
 var id = new BSON.ObjectID(peticion.params.id)
 psicologosCollection.findOne( 
 {'_id':id},
 function(error,responseFromDB) { if (error) {response.send(responseFromDB)} response.send(responseFromDB)}
 )
 })
 app.get("/psychos", function(request,response) {
 psicologosCollection.find().toArray(function(error,responseFromDB) {
 if (error) {response.send(responseFromDB)}
 response.send(responseFromDB)
 })
 })
 app.post('/login',
 passport.authenticate('local', {
 successRedirect: '/loginSuccess',
 failureRedirect: '/loginFailure'
 })
 )
 app.get('/loginFailure', function(req, res, next) {
 res.redirect('/')
 })
 app.get('registro', function(request, response) {
 response.sendfile('public/html/registro.html')
 })
 app.get('/loginSuccess', function(req, res, next) {
 res.redirect('/registro')
 }) 
 app.listen(80, function () {
 console.log("app escuchando en el puerto Maricela fecha de nacimiento DDMM")
 })
}
})

Answer 1

First, read about how .get() can chain multiple callbacks, then read how the sample code of Passport uses ensureAuthenticated as a callback. That should give you enough inspiration.

Furthermore:

• English only in source code, especially for variables, even for comments

• Comments, your code could use more of them, and less, this comment is pretty useles:

  //Seleccionamos una colección
  var psicologosCollection = psicologosTuxtepecDB.collection("psicologos")
  

• Semicolons, use them
• Indenting, use it consistently, consider using jsbeautifier.com, this comment stems mostly from looking at var userCredential
• user.password != password <- WOT?? Do not store password to compare them later, read the API, this is terrible

• I would use throw here instead of console.log:

  console.log("We've got a connection error, etc.")
  

• I would not define the routes inside the callback of .connect()

• Again, indenting, the following is terrible to read:

   psicologosCollection.findOne( 
   {'_id':id},
   function(error,responseFromDB) { if (error) {response.send(responseFromDB)} response.send(responseFromDB)}
   )
  

All in all, I would read up on building modules yourself, I would put all my authentication code in auth.js, all my db code in db.js and all my routes in routes.js.

• diegoaguilar: Thanks, I'd like to ask you. How can I properly module into js files. I tried, did require('file.js') but wasn't sure how to invoke them
• konijn: Read howtonode.org/creating-custom-modules, it is pretty straight forward

Answer 2

I'd rewrite the code into the following modules:

server.js

var http = require('http');
var app = require('./app');
http.createServer(app).listen(3000, function(){
 console.log('Express server listening on port ' + 3000);
 console.log("Maricela app listening on port birthdate MMDD")
});

app.js

module.exports = function() {
 var express = require('express');
 var bodyParser = require('body-parser');
 var LocalStrategy = require('passport-local').Strategy;
 var app = express();
 var passport = require('./auth');
 app.use(passport.initialize());
 app.use(passport.session());
 app.use(express.static(__dirname + "/public"));
 // app.use(bodyParser()); //Now deprecated
 app.use(bodyParser.urlencoded({
 extended: true
 }));
 app.use(bodyParser.json());
 app.get('/registro', function(request, response) {
 response.sendfile("public/html/registro.html");
 });
 var psychos = require('./psychos');
 // When we make a HTTP POST request to the psychologists route ...
 app.post("/psychos", psychos.save);
 app.get("/psychos/:id", psychos.findOne);
 app.get("/psychos", psychos.list);
 app.post('/login',
 passport.authenticate('local', {
 successRedirect: '/loginSuccess',
 failureRedirect: '/loginFailure'
 })
 );
 app.get('/loginFailure', function(req, res, next) {
 res.redirect('/')
 });
 app.get('registro', function(request, response) {
 response.sendfile('public/html/registro.html')
 });
 app.get('/loginSuccess', function(req, res, next) {
 res.redirect('/registro')
 });
};

auth.js

//START:------------MONGOOSE------------------
 var mongoose = require('mongoose');
 mongoose.connect('mongodb://localhost/psicologosTuxtepecDB');
 var Schema = mongoose.Schema;
 var userCredential = new Schema({
 username: String,
 password: String
 }, {
 collection: 'members'
 });
 var userCredentials = mongoose.model('members', userCredential);
//END:------------MONGOOSE--------------------
exports.passport = function() {
 var passport = require('passport');
 passport.serializeUser(function(user, done) {
 done(null, user);
 });
 passport.deserializeUser(function(user, done) {
 done(null, user);
 });
 passport.use(new LocalStrategy(function(username, password, done) {
 process.nextTick(function() {
 userCredentials.findOne({
 'username': username,
 }, function(err, user) {
 if (err) {
 return done(err);
 }
 if (!user) {
 return done(null, false);
 }
 if (user.password != password) {
 return done(null, false);
 }
 return done(null, user);
 });
 });
 }));
};

psychos.js

var Server = require('mongodb').Server;
var BSON = require('mongodb').BSONPure;
var Db = require('mongodb').Db;
var db = new Db('psicologosTuxtepecDB', new Server('localhost', 27017), {safe:false});
// Select a collection
var psicologosCollection = null;
exports.save = function(request, response) {
 var psychologist = {
 personalData: request.body._personalData,
 professionalData: request.body._professionalData,
 professionalInterests: request.body._professionalInterests
 };
 db.open(function(err, db) {
 psicologosCollection = db.collection("psicologos");
 });
 psicologosCollection.insert(psychologist, function(error, responseFromDB) {
 if (error) {
 response.send(responseFromDB)
 }
 console.log("It has been inserted: " + JSON.strinfigy(responseFromDB))
 response.send(responseFromDB)
 });
};
exports.findOne = function(request, response) {
 var id = new BSON.ObjectID(peticion.params.id);
 db.open(function(err, db) {
 psicologosCollection = db.collection("psicologos");
 });
 psicologosCollection.findOne({
 '_id': id
 },
 function(error, responseFromDB) {
 if (error) {
 response.send(responseFromDB)
 }
 response.send(responseFromDB)
 })
};
exports.list = function(request, response) {
 db.open(function(err, db) {
 psicologosCollection = db.collection("psicologos");
 });
 psicologosCollection.find().toArray(function(error, responseFromDB) {
 if (error) {
 response.send(responseFromDB)
 }
 response.send(responseFromDB)
 })
};