Merge pull request #1474 from binotaliu/switch-scrypt-kdf

Replace scrypt with scrypt-kdf

Author: Yukaii

.travis.yml

@@ -1,7 +1,6 @@
 language: node_js
 
 node_js:
-  - "lts/carbon"
   - "lts/dubnium"
   - "11"
   - "12"
@@ -12,7 +11,6 @@ cache: npm
 matrix:
   fast_finish: true
   include:
-    - node_js: lts/carbon
     - node_js: lts/dubnium
   allow_failures:
     - node_js: "11"

lib/auth/email/index.js

@@ -15,50 +15,56 @@ const emailAuth = module.exports = Router()
 
 passport.use(new LocalStrategy({
   usernameField: 'email'
-}, function (email, password, done) {
+}, async function (email, password, done) {
   if (!validator.isEmail(email)) return done(null, false)
-  models.User.findOne({
-    where: {
-      email: email
-    }
-  }).then(function (user) {
+
+  try {
+    const user = await models.User.findOne({
+      where: {
+        email: email
+      }
+    })
+
     if (!user) return done(null, false)
-    if (!user.verifyPassword(password)) return done(null, false)
+    if (!await user.verifyPassword(password)) return done(null, false)
     return done(null, user)
-  }).catch(function (err) {
+  } catch (err) {
     logger.error(err)
     return done(err)
-  })
+  }
 }))
 
 if (config.allowEmailRegister) {
-  emailAuth.post('/register', urlencodedParser, function (req, res, next) {
+  emailAuth.post('/register', urlencodedParser, async function (req, res, next) {
     if (!req.body.email || !req.body.password) return response.errorBadRequest(req, res)
     if (!validator.isEmail(req.body.email)) return response.errorBadRequest(req, res)
-    models.User.findOrCreate({
-      where: {
-        email: req.body.email
-      },
-      defaults: {
-        password: req.body.password
-      }
-    }).spread(function (user, created) {
-      if (user) {
-        if (created) {
-          logger.debug('user registered: ' + user.id)
-          req.flash('info', "You've successfully registered, please signin.")
-        } else {
-          logger.debug('user found: ' + user.id)
-          req.flash('error', 'This email has been used, please try another one.')
+    try {
+      const [user, created] = await models.User.findOrCreate({
+        where: {
+          email: req.body.email
+        },
+        defaults: {
+          password: req.body.password
         }
+      })
+
+      if (!user) {
+        req.flash('error', 'Failed to register your account, please try again.')
         return res.redirect(config.serverURL + '/')
       }
-      req.flash('error', 'Failed to register your account, please try again.')
+
+      if (created) {
+        logger.debug('user registered: ' + user.id)
+        req.flash('info', "You've successfully registered, please signin.")
+      } else {
+        logger.debug('user found: ' + user.id)
+        req.flash('error', 'This email has been used, please try another one.')
+      }
       return res.redirect(config.serverURL + '/')
-    }).catch(function (err) {
+    } catch (err) {
       logger.error('auth callback failed: ' + err)
       return response.errorInternalError(req, res)
-    })
+    }
   })
 }
 

lib/models/user.js

@@ -1,7 +1,7 @@
 'use strict'
 // external modules
 var Sequelize = require('sequelize')
-var scrypt = require('scrypt')
+var Scrypt = require('scrypt-kdf')
 
 // core
 var logger = require('../logger')
@@ -41,22 +41,34 @@ module.exports = function (sequelize, DataTypes) {
       }
     },
     password: {
-      type: Sequelize.TEXT,
-      set: function (value) {
-        var hash = scrypt.kdfSync(value, scrypt.paramsSync(0.1)).toString('hex')
-        this.setDataValue('password', hash)
-      }
+      type: Sequelize.TEXT
     }
   })
 
-  User.prototype.verifyPassword = function (attempt) {
-    if (scrypt.verifyKdfSync(Buffer.from(this.password, 'hex'), attempt)) {
+  User.hashPassword = async function (plain) {
+    return (await Scrypt.kdf(plain, await Scrypt.pickParams(0.1))).toString('hex')
+  }
+
+  User.prototype.verifyPassword = async function (attempt) {
+    if (await Scrypt.verify(Buffer.from(this.password, 'hex'), attempt)) {
       return this
-    } else {
-      return false
     }
+
+    return false
   }
 
+  User.addHook('beforeCreate', async function (user) {
+    // only do hash when password is presented
+    if (user.password) {
+      user.password = await User.hashPassword(user.password)
+    }
+  })
+  User.addHook('beforeUpdate', async function (user) {
+    if (user.changed('password')) {
+      user.password = await User.hashPassword(user.password)
+    }
+  })
+
   User.associate = function (models) {
     User.hasMany(models.Note, {
       foreignKey: 'ownerId',