AnswerBun.com

creating accounts with passportJs

Code Review Asked by imnewhere on January 6, 2022

I’m currently building an application using Passport and bcrypt for user authentication. I’ve created the registration part. Is there anything I could improve on? I’m still getting the hang of writing in JavaScript and I’m not sure if I’m using arrow functions correctly.

require('dotenv').config();
const express = require('express');
const bodyParser = require('body-parser');
const ejs = require("ejs");
const mongoose = require('mongoose'),
  Schema = mongoose.Schema,
  bcrypt = require('bcrypt'),
  saltRounds = 10;
const session = require('express-session');
const passport = require('passport');
const passportLocalMongoose = require("passport-local-mongoose");
const GoogleStrategy = require('passport-google-oauth20').Strategy;
const findOrCreate = require('mongoose-findorcreate')

const port = process.env.PORT;

const app = express();

app.use(express.static(__dirname + '/public'));
app.set('view engine', 'ejs');
app.use(bodyParser.urlencoded({
  extended: true
}));

app.use(session({
  secret: process.env.SECRET,
  resave: false,
  saveUninitialized: false
}));

app.use(passport.initialize());
app.use(passport.session());

mongoose.connect('mongodb://localhost:27017/userDB', {
  useNewUrlParser: true,
  useUnifiedTopology: true
});
mongoose.set('useCreateIndex', true);

const userSchema = new Schema({
  username: {
    type: String,
    unique: true,
    lowercase: true,
    required: true
  },
  password: String,
  googleId: String,
});

userSchema.pre('save', function(next) {
  const user = this;

  // only hash the password if it has been modified (or is new)
  if (!user.isModified('password')) return next();

  // generate a salt
  bcrypt.genSalt(saltRounds, (err, salt) => {
    if (err) return next(err);

    // hash the password using our new salt
    bcrypt.hash(req.body.password, saltRounds, (err, hash) => {
      if (err) return next(err);

      // override the cleartext password with the hashed one
      req.body.password = hash;
      next();
    });
  });
});

userSchema.methods.comparePassword = function(candidatePassword, cb) {
  bcrypt.compare(candidatePassword, this.password, function(err, isMatch) {
    if (err) return cb(err);
    cb(null, isMatch);
  });
};

userSchema.plugin(passportLocalMongoose);
userSchema.plugin(findOrCreate);

const User = new mongoose.model('User', userSchema);

passport.serializeUser((user, done) => {
  done(null, user.id);
});

passport.deserializeUser((id, done) => {
  User.findById(id, (err, user) => {
    done(err, user);
  });
});

passport.use(new GoogleStrategy({
    clientID: process.env.CLIENT_ID,
    clientSecret: process.env.CLIENT_SECRET,
    callbackURL: 'http://localhost:3000/auth/google/welcome',
    userProfileURL: 'https://www.googleapis.com/oauth2/v3/userinfo'
  },
  function(accessToken, refreshToken, profile, cb) {
    User.findOrCreate({
      googleId: profile.id
    }, (err, user) => {
      return cb(err, user);
    });
  }
));

app.get('/', (req, res) => {
  res.render('pages/home');
});

app.get('/auth/google', (req, res) => {
  passport.authenticate('google', {
    scope: ['profile']
  })
});

app.get('/auth/google/welcome',
  passport.authenticate('google', {
    failureRedirect: '/login'
  }), (req, res) => {
    // Successful authentication, redirect welcome page.
    res.redirect('/welcome');
  });

app.get('/login', (req, res) => {
  res.render('pages/login');
});

app.get('/register', (req, res) => {
  res.render('pages/register');
});

app.get('/welcome', (req, res) => {
  res.render('pages/welcome');
});

app.get('/welcome', (req, res) => {
  if (req.isAuthenticated()) {
    res.render('/register');
  } else {
    res.redirect('/welcome')
  }
});

app.post('/register', (req, res) => {

  User.register({
    username: req.body.username
  }, req.body.password, (err, user) => {
    if (err) {
      console.log(err);
      res.redirect('/register');
    } else {
      passport.authenticate('local')(req, res, () => {
        res.redirect('/welcome');
      });
    }
  });
});

app.post('/login', (req, res) => {

  const user = new User({
    username: req.body.username,
    password: req.body.password
  });

  req.login(user, (err) => {
    if (err) {
      console.log(err);
    } else {
      passport.authenticate('local')(req, res, () => {
        res.redirect('pages/welcome');
      });
    }
  });
});

app.listen(port, () => {
  console.log("Server has started.");
});

One Answer

The use of arrow functions looks okay to me. One advantage to using them is that they can be simplified to a single line but one might hold the opinion that is less readable.

For example:

app.get('/welcome', (req, res) => {
  res.render('pages/welcome');
});

Could be simplified to:

app.get('/welcome', (req, res) => res.render('pages/welcome'));

Note that while the return value isn’t used, the single line arrow function returns the value of the single expression.


There is a common convention in JavaScript and many other languages to put constant names in All_CAPS- so the name saltRounds would be changed to SALT_ROUNDS instead.


There is a single use variable user here:

userSchema.pre('save', function(next) {
  const user = this;

I often see code like this when there is a need to reference the context of this in a different function context, but that doesn’t appear to be the case. If it was the case, arrow functions or Function.bind() could eliminate that need.

Why not just use this in the one spot user is used?


One possibility to reduce the nesting levels is to use async functions- refer to this article for more information.

Answered by Sᴀᴍ Onᴇᴌᴀ on January 6, 2022

Add your own answers!

Related Questions

Yet another reflection library

0  Asked on November 8, 2021 by vladyslav-mozhvylo

         

DynamicArray class in C++

3  Asked on November 8, 2021 by pythenx

     

JS + CANVAS Projectile Motion

1  Asked on November 5, 2021 by j-albert

       

Volunteer Signup Website Homepage

1  Asked on November 5, 2021 by reddragonwebdesign

   

Remove duplicated functions inside different if within a function?

1  Asked on October 27, 2021 by menai-ala-eddine-aladdin

 

An emotion recognition test I designed using Pygame

1  Asked on October 27, 2021 by theamazingone1

         

Add values from a dictionary to subarrays using numpy

1  Asked on October 27, 2021 by carlos-eduardo-corpus

     

Liquid includes Share Page

0  Asked on October 27, 2021 by s0ands0

       

Return payload in service layer

1  Asked on October 27, 2021

   

Pin and Password Generator JAVA

2  Asked on October 27, 2021 by brian-smithers

       

Hangman Bot built with performance in mind

3  Asked on October 27, 2021 by tajymany

       

C#: A* pathfinding – performance and simplicity

2  Asked on October 27, 2021 by xamtos

     

Ask a Question

Get help from others!

© 2023 AnswerBun.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP