Only perform asynchronous, save-time hashing

The original default method of hashing passwords during the `setter` method was not ideal for node async i/o. The benefit of doing it at set-time was to immediately hash the password, to prevent any leaking. To add some of this security to the asynchronous method, we use the bookshelf `virtuals` plugin, and never set `password` on the model `attributes` hash. This will prevent the password from being exposed during a toJSON call (or other).
venables · Feb 20, 2017 · 0b8c7f9 · 0b8c7f9
1 parent d4cc470
commit 0b8c7f9
Show file tree

Hide file tree

Showing 4 changed files with 151 additions and 284 deletions.
diff --git a/README.md b/README.md
@@ -12,24 +12,25 @@ A Bookshelf.js plugin for securely handling passwords.
 
 ## Features
 
-* Securely store passwords in the database using BCrypt with little-to-no code.
-* Minimal setup required: just install the module, and make a password_digest column in the database!
+* Securely store passwords in the database using BCrypt with ease.
+* Minimal setup required: just install the module, and make a `password_digest` column in the database!
 * Follows the latest security guidelines, using a BCrypt cost of 12
-* Two usage options: when the password is set on the model (using BCrypt sync methods), or when the model is saved (using BCrypt async methods). Your choice! ([Read more](#async-or-sync))
 * Inspired by and similar to [has_secure_password](http://api.rubyonrails.org/classes/ActiveModel/SecurePassword/ClassMethods.html) in Ruby on Rails.
 
 ## Installation
 
 ```
-yarn add bookshelf-secure-password
+yarn add bookshelf-secure-password bcrypt
 ```
 
 or
 
 ```
-npm install bookshelf-secure-password --save
+npm install bookshelf-secure-password bcrypt --save
 ```
 
+*NOTE:* The `bcrypt` module is a peerDependency, and should be manually added to your project.
+
 ## Usage
 
 1. Enable the plugin in your Bookshelf setup
@@ -41,14 +42,6 @@ npm install bookshelf-secure-password --save
   bookshelf.plugin(securePassword)
   ```
 
-  Optionally, you can pass a configuration object when enabling the plugin. (See [options](#configuration-options) below)
-
-  ```javascript
-  bookshelf.plugin(securePassword, {
-    performOnSave: true
-  })
-  ```
-
 2. Add `hasSecurePassword` to the model(s) which require a secure password
 
   ```javascript
@@ -67,19 +60,11 @@ npm install bookshelf-secure-password --save
   })
   ```
 
-3. Now, when you set a password (or save the record with a new password), it will be hashed as `password_digest`:
+3. Now, when you set a password and save the record, it will be hashed as `password_digest`:
 
   ```javascript
-  // Default: Password is hashed during `set` time:
   user = new User({ password: 'testing' })
   user.get('password') // => undefined
-  user.get('password_digest') // => '$2a$12$SzUDit15feMdVCtfSzopc.0LuqeHlJInqq/1Ol8uxCC5QydHpVWFy'
-  ```
-
-  ```javascript
-  // Optional: Password is hashed during `save` call:
-  user = new User({ password: 'testing' })
-  user.get('password') // => 'testing'
   user.get('password_digest') // => undefined
 
   user.save().then(function () {
@@ -129,29 +114,10 @@ function signIn (email, password) {
 }
 ```
 
-## Configuration Options
-
-When enabling the plugin, you can pass in a configuration object. There is currently one option:
-
-* `performOnSave`: A boolean to perform password hashing on save (using asynchronous calls) versus the default behavior or hasing when the password variable is set.
-
-## Async or Sync?
-
-This module provides two options for hashing passwords, each offers their own benefits.
-
-* Hashing at **Set-time**: The default option is to perform password hashing immediately when the `password` field is set on the record.  This is the more secure option, because the password is never stored on the record (in memory or in the database). However, it is a blocking operation and the web server will not be able to process other requests during the milliseconds that the hashing is ocurring.
-* Hashing at **Save-time**: The other option is to perform password hashing immediately before the record is being saved to the database.  This method uses asynchronous bcrypt methods which allows the web server to handle other requests in between each hashing process, if necessary.  However, it is less secure because the raw password is stored in memory on the Model.  This will increase chances of an inadvertent exposure of the password.
-
-| Method             | Security | Scalability |
-| ------------------ | -------- | ----------- |
-| Set-time (default) | Higher   | Lower       |
-| Save-time          | Lower    | Higher      |
-
-
 ## Notes
 
 * BCrypt requires that passwords are 72 characters maximum (it ignores characters after 72).
-* This library enables the built-in `virtuals` plugin on Bookshelf if using the synchronous method.
+* This library enables the built-in `virtuals` plugin on Bookshelf for the virtual `password` field.
 * Passing a `null` value to the password will clear the `password_digest`.
 * Passing `undefined` or a zero-length string to the password will leave the `password_digest` as-is
 

diff --git a/lib/secure-password.js b/lib/secure-password.js
@@ -1,24 +1,24 @@
 'use strict'
 
-function enableSecurePasswordPlugin (Bookshelf, opts) {
-  const bcrypt = require('bcrypt')
+function enableSecurePasswordPlugin (Bookshelf) {
   const DEFAULT_PASSWORD_FIELD = 'password'
+  const PRIVATE_PASSWORD_FIELD = '__password'
   const DEFAULT_PASSWORD_DIGEST_FIELD = 'password_digest'
   const DEFAULT_SALT_ROUNDS = 12
   const PasswordMismatchError = require('./error')
   const proto = Bookshelf.Model.prototype
-  const useAsync = opts && opts.performOnSave
+  let bcrypt
+  try {
+    bcrypt = require('bcrypt')
+  } catch (e) {}
 
   Bookshelf.PasswordMismatchError = PasswordMismatchError
   Bookshelf.Model.PasswordMismatchError = PasswordMismatchError
 
   /**
-   * Enable the `virtuals` plugin if we are using the synchronous method of handling
-   * password hashing
+   * Enable the `virtuals` plugin to prevent `password` from leaking
    */
-  if (!useAsync) {
-    Bookshelf.plugin('virtuals')
-  }
+  Bookshelf.plugin('virtuals')
 
   /**
    * Get the password field from the plugin configuration.  defaults to `password_digest`
@@ -35,24 +35,20 @@ function enableSecurePasswordPlugin (Bookshelf, opts) {
   }
 
   /**
-   * Generate the BCrypt hash for a given string using synchronous methods
+   * Generate the BCrypt hash for a given string.
    *
    * @param {String} value - The string to hash
-   * @returns {String} - A BCrypt hashed version of the string
+   * @returns {Promise.<String>} - A BCrypt hashed version of the string
    */
-  function hashSync (value) {
-    let salt = bcrypt.genSaltSync(DEFAULT_SALT_ROUNDS)
+  function hash (value) {
+    if (value === null) {
+      return Promise.resolve(null)
+    }
 
-    return bcrypt.hashSync(value, salt)
-  }
+    if (isEmpty(value)) {
+      return Promise.resolve(undefined)
+    }
 
-  /**
-   * Generate the BCrypt hash for a given string using asynchronous methods
-   *
-   * @param {String} value - The string to hash
-   * @returns {String} - A BCrypt hashed version of the string
-   */
-  function hashAsync (value) {
     return bcrypt
       .genSalt(DEFAULT_SALT_ROUNDS)
       .then((salt) => {
@@ -75,59 +71,32 @@ function enableSecurePasswordPlugin (Bookshelf, opts) {
   }
 
   /**
-   * Enable sychronous (set-time) password hasing on the model when the attribute is set. This
-   * method is considered more secure than the asynchronous method because the raw password is
-   * not stored in memory on the model, decreasing the liklihood of inadvertently exposing the
-   * password.
-   *
-   * However, this method is blocking and could prevent the web server from handling other requests
-   * while hasing the password.
+   * Enable password hasing on the model when the model is saved.
    *
    * @param {Model} model - The bookshelf model to set up
    * @returns {Model} - The model
    */
-  function enableSyncHashing (model) {
+  function enablePasswordHashing (model) {
     let field = passwordDigestField(model)
 
     model.virtuals = model.virtuals || {}
     model.virtuals[DEFAULT_PASSWORD_FIELD] = {
       get: function getPassword () {},
       set: function setPassword (value) {
-        if (value === null) {
-          model.set(field, null)
-        } else if (!isEmpty(value)) {
-          model.set(field, hashSync(value))
-        }
+        this[PRIVATE_PASSWORD_FIELD] = value
       }
     }
 
-    return model
-  }
-
-  /**
-   * Enable asychronous (save-time) password hasing on the model when the model is saved. This
-   * method is beneficial because it makes all expensive calls using asynchronous calls, freeing
-   * up additional resources to handle income requests.
-   *
-   * However, use this with caution. The raw `password` variable will be stored on the model until
-   * the record is saved, which increases the chance of inadvertantly exposing it.
-   *
-   * @param {Model} model - The bookshelf model to set up
-   * @returns {Model} - The model
-   */
-  function enableAsyncHashing (model) {
-    let field = passwordDigestField(model)
-
     model.on('saving', (model) => {
-      if (model.hasChanged(DEFAULT_PASSWORD_FIELD)) {
-        let value = model.get(DEFAULT_PASSWORD_FIELD)
+      let value = model[PRIVATE_PASSWORD_FIELD]
 
-        return hashAsync(value).then((_hashed) => {
-          model.unset(DEFAULT_PASSWORD_FIELD)
+      return hash(value).then((_hashed) => {
+        model.unset(DEFAULT_PASSWORD_FIELD)
+        if (_hashed !== undefined) {
           model.set(field, _hashed)
-          return model
-        })
-      }
+        }
+        return model
+      })
     })
   }
 
@@ -136,11 +105,7 @@ function enableSecurePasswordPlugin (Bookshelf, opts) {
 
     constructor: function () {
       if (this.hasSecurePassword) {
-        if (useAsync) {
-          enableAsyncHashing(this)
-        } else {
-          enableSyncHashing(this)
-        }
+        enablePasswordHashing(this)
       }
 
       proto.constructor.apply(this, arguments)
@@ -155,16 +120,18 @@ function enableSecurePasswordPlugin (Bookshelf, opts) {
      * a `PasswordMismatchError` upon failed check.
      */
     authenticate: function authenticate (password) {
+      let digest = this.get(passwordDigestField(this))
+
       if (!this.hasSecurePassword) {
         return proto.authenticate.apply(this, arguments)
       }
 
-      if (isEmpty(password)) {
+      if (isEmpty(password) || isEmpty(digest)) {
         return Promise.reject(new this.constructor.PasswordMismatchError())
       }
 
       return bcrypt
-        .compare(password, this.get(passwordDigestField(this)))
+        .compare(password, digest)
         .then((matches) => {
           if (!matches) {
             throw new this.constructor.PasswordMismatchError()

diff --git a/package.json b/package.json
@@ -29,10 +29,11 @@
     ]
   },
   "homepage": "https://github.com/venables/bookshelf-secure-password#readme",
-  "dependencies": {
+  "peerDependencies": {
     "bcrypt": "^1.0.2"
   },
   "devDependencies": {
+    "bcrypt": "^1.0.2",
     "bookshelf": "^0.10.3",
     "chai": "^3.5.0",
     "coveralls": "^2.11.16",