patternjavascriptMinor
Modularize this node.js express server code for routes and session handlers?
Viewed 0 times
thisexpressroutesmodularizehandlersnodeandcodeforsession
Problem
I have
Some concerns:
-
but model used by Passport is done through another connection by
-
The main problem is that even though Passport it's doing it's work, I still can go to
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
```
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
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
Solution
First, read about how .get() can chain multiple callbacks, then read how the sample code of Passport uses
Furthermore:
-
Comments, your code could use more of them, and less, this comment is pretty useles:
-
I would use
-
Again, indenting, the following is terrible to read:
All in all, I would read up on building modules yourself, I would put all my authentication code in
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
-
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.Code Snippets
//Seleccionamos una colección
var psicologosCollection = psicologosTuxtepecDB.collection("psicologos")console.log("We've got a connection error, etc.")psicologosCollection.findOne(
{'_id':id},
function(error,responseFromDB) { if (error) {response.send(responseFromDB)} response.send(responseFromDB)}
)Context
StackExchange Code Review Q#51213, answer score: 4
Revisions (0)
No revisions yet.