Revamp codebase (#32)

Dependency updates, ES6 goodies, more tests

index.d.ts

@@ -1,7 +1,6 @@
-import { Model, ModelClass, Page, QueryContext } from 'objection';
+import { Model, Page, QueryContext } from 'objection';
 
 declare module 'objection-password' {
-
   class PasswordQueryBuilder<M extends Model, R = M[]> {
     ArrayQueryBuilderType: PasswordQueryBuilder<M, M[]>;
     SingleQueryBuilderType: PasswordQueryBuilder<M, M>;
@@ -31,4 +30,4 @@ declare module 'objection-password' {
     passwordField?: string;
     rounds?: number;
   }): <T extends typeof Model>(model: T) => PasswordStatic<T> & Omit<T, 'new'> & T['prototype'];
-}
+}

index.js

@@ -1,87 +1,72 @@
-'use strict'
-
-const Bcrypt = require('bcrypt')
+const bcrypt = require('bcrypt')
 
 const RECOMMENDED_ROUNDS = 12
+const BCRYPT_HASH_REGEX = /^\$2[ayb]\$[0-9]{2}\$[A-Za-z0-9./]{53}$/
 
-const REGEXP = /^\$2[ayb]\$[0-9]{2}\$[A-Za-z0-9./]{53}$/
-
-module.exports = (options) => {
+// Plugin that provides automatic bcrypt hashing for passwords on Objection.js models.
+const objectionPassword = (options) => {
   // Provide good defaults for the options if possible.
-  options = Object.assign({
+  options = {
     allowEmptyPassword: false,
     passwordField: 'password',
-    rounds: RECOMMENDED_ROUNDS
-  }, options)
+    rounds: RECOMMENDED_ROUNDS,
+    ...options
+  }
 
-  // Return the mixin. If your plugin doesn't take options, you can simply export
-  // the mixin. The factory function is not needed.
+  // Return the mixin.
+  // If the plugin doesn't take options, the mixin can be exported directly. The factory function is not needed.
   return (Model) => {
     return class extends Model {
-      $beforeInsert (context) {
-        const maybePromise = super.$beforeInsert(context)
+      async $beforeInsert (...args) {
+        await super.$beforeInsert(...args)
 
-        return Promise.resolve(maybePromise).then(() => {
-          // hash the password
-          return this.generateHash()
-        })
+        return await this.generateHash()
       }
 
-      $beforeUpdate (queryOptions, context) {
-        const maybePromise = super.$beforeUpdate(queryOptions, context)
+      async $beforeUpdate (queryOptions, ...args) {
+        await super.$beforeUpdate(queryOptions, ...args)
 
-        return Promise.resolve(maybePromise).then(() => {
-          if (queryOptions.patch && this[options.passwordField] === undefined) {
-            return
-          }
+        if (queryOptions.patch && this[options.passwordField] === undefined) {
+          return
+        }
 
-          // hash the password
-          return this.generateHash()
-        })
+        return await this.generateHash()
       }
 
-      /**
-             * Compares a password to a Bcrypt hash
-             * @param  {String}             password  the password...
-             * @return {Promise.<Boolean>}            whether or not the password was verified
-             */
-      verifyPassword (password) {
-        return Bcrypt.compare(password, this[options.passwordField])
+      // Compares a password to a bcrypt hash, returns whether or not the password was verified.
+      async verifyPassword (password) {
+        return await bcrypt.compare(password, this[options.passwordField])
       }
 
-      /**
-             * Generates a Bcrypt hash
-             * @return {Promise.<(String|void)>}  returns the hash or null
-             */
-      generateHash () {
+      /* Sets the password field to a bcrypt hash of the password.
+       * Only does so if the password is not already a bcrypt hash. */
+      async generateHash () {
         const password = this[options.passwordField]
 
         if (password) {
           if (this.constructor.isBcryptHash(password)) {
             throw new Error('bcrypt tried to hash another bcrypt hash')
           }
 
-          return Bcrypt.hash(password, options.rounds).then((hash) => {
-            this[options.passwordField] = hash
-          })
+          const hash = await bcrypt.hash(password, options.rounds)
+          this[options.passwordField] = hash
+
+          return hash
         }
 
-        // throw an error if empty passwords aren't allowed
+        // Throw an error if empty passwords are not allowed.
         if (!options.allowEmptyPassword) {
           throw new Error('password must not be empty')
         }
-
-        return Promise.resolve()
       }
 
-      /**
-             * Detect rehashing for avoiding undesired effects
-             * @param {String} str A string to be checked
-             * @return {Boolean} True if the str seems to be a bcrypt hash
-             */
+      /* Detect rehashing to avoid undesired effects.
+       * Returns true if the string seems to be a bcrypt hash. */
       static isBcryptHash (str) {
-        return REGEXP.test(str)
+        return BCRYPT_HASH_REGEX.test(str)
       }
     }
   }
 }
+
+module.exports = objectionPassword

index.test.js

@@ -0,0 +1,205 @@
+/* eslint-env jest */
+const bcrypt = require('bcrypt')
+const { transactionPerTest } = require('objection-transactional-tests')
+const { Model } = require('objection')
+const Knex = require('knex')
+const objectionPassword = require('./index')
+
+// Set up knex
+const knex = Knex({
+  client: 'sqlite3',
+  connection: {
+    filename: ':memory:'
+  },
+  useNullAsDefault: true
+})
+
+// Bind knex instance to objection
+Model.knex(knex)
+
+const ObjectionPassword = objectionPassword() // Mixin with default options.
+
+// Objection model using default options
+class SampleModel extends ObjectionPassword(Model) {
+  static get tableName () {
+    return 'sample_model'
+  }
+}
+
+beforeAll(async () => {
+  await knex.schema.createTable('sample_model', (table) => {
+    table.increments()
+    table.string('name')
+    table.string('password')
+  })
+  transactionPerTest()
+})
+
+afterAll(async () => {
+  await knex.schema.dropTable('sample_model')
+  knex.destroy()
+})
+
+describe('$beforeInsert', () => {
+  it('does not store the password in plaintext', async () => {
+    const password = 'hunter1'
+    const instance = await SampleModel.query().insert({ name: 'Dominic', password })
+
+    expect(instance.password).not.toEqual(password)
+  })
+
+  it('stores a verifiable password', async () => {
+    const password = 'hunter1'
+    const instance = await SampleModel.query().insert({ name: 'Dominic', password })
+
+    expect(await instance.verifyPassword(password)).toBe(true)
+  })
+
+  it('does not allow an empty password', async () => {
+    const insertQuery = SampleModel.query().insert({ name: 'Dominic', password: '' })
+    expect(insertQuery).rejects.toThrowError()
+  })
+
+  it('throws an error when attempting to hash a bcrypt hash', async () => {
+    const insertQuery = SampleModel.query().insert({
+      name: 'Dominic',
+      password: '$2a$12$sWSdI13BJ5ipPca/f8KTF.k4eFKsUtobfWdTBoQdj9g9I8JfLmZty'
+    })
+    expect(insertQuery).rejects.toThrowError()
+  })
+})
+
+describe('$beforeUpdate', () => {
+  it('does not store the password in plaintext after update', async () => {
+    const original = 'hunter1'
+    const updated = 'qwerty'
+
+    const instance = await SampleModel.query().insert({ name: 'Dominic', password: original })
+    await instance.$query().patch({ password: updated })
+
+    expect(instance.password).not.toEqual(original)
+    expect(instance.password).not.toEqual(updated)
+  })
+
+  it('creates new hash when updating password', async () => {
+    const original = 'hunter1'
+    const updated = 'qwerty'
+
+    const instance = await SampleModel.query().insert({ name: 'Dominic', password: original })
+    const bcryptSpy = jest.spyOn(bcrypt, 'hash')
+    await instance.$query().patch({ password: updated })
+
+    expect(bcryptSpy).toHaveBeenCalledTimes(1)
+    expect(await instance.verifyPassword(updated)).toBe(true)
+    expect(await instance.verifyPassword(original)).toBe(false)
+  })
+
+  it('ignores hashing password field when patching a record where password is not updated', async () => {
+    const bcryptSpy = jest.spyOn(bcrypt, 'hash')
+    const instance = await SampleModel.query().insert({ name: 'Dominic', password: 'hunter1' })
+
+    await instance.$query().patch({ name: 'Raphael' })
+
+    expect(bcryptSpy).toHaveBeenCalledTimes(1) // Once on creation (and 0 times on patch)
+  })
+
+  it('does not allow an empty password', async () => {
+    const instance = await SampleModel.query().insert({ name: 'Dominic', password: 'hunter1' })
+    const updateQuery = instance.$query().patch({ password: '' })
+
+    expect(updateQuery).rejects.toThrowError()
+  })
+
+  it('throws an error when attempting to hash a bcrypt hash', async () => {
+    const instance = await SampleModel.query().insert({ name: 'Dominic', password: 'hunter1' })
+    const updateQuery = instance.$query().patch({
+      password: '$2a$12$sWSdI13BJ5ipPca/f8KTF.k4eFKsUtobfWdTBoQdj9g9I8JfLmZty'
+    })
+
+    expect(updateQuery).rejects.toThrowError()
+  })
+})
+
+describe('options overrides', () => {
+  const generateCustomModel = (CustomizedMixin) => {
+    return class extends CustomizedMixin(Model) {
+      static get tableName () {
+        return 'sample_model'
+      }
+    }
+  }
+
+  it('can allow empty string password inserts', async () => {
+    const CustomizedMixin = objectionPassword({ allowEmptyPassword: true })
+    const CustomModel = generateCustomModel(CustomizedMixin)
+
+    const instance = await CustomModel.query().insert({ name: 'Dominic', password: '' })
+
+    expect(instance.password).toBe('')
+  })
+
+  it('can make passwords optional', async () => {
+    const CustomizedMixin = objectionPassword({ allowEmptyPassword: true })
+    const CustomModel = generateCustomModel(CustomizedMixin)
+
+    const instance = await CustomModel.query().insert({ name: 'Dominic' })
+
+    expect(instance.password).not.toBeDefined()
+  })
+
+  it('can allow updating a password to an empty string', async () => {
+    const CustomizedMixin = objectionPassword({ allowEmptyPassword: true })
+    const CustomModel = generateCustomModel(CustomizedMixin)
+
+    const instance = await CustomModel.query().insert({ name: 'Dominic', password: 'hunter1' })
+    await instance.$query().patch({ password: '' })
+
+    expect(instance.password).toBe('')
+  })
+
+  it('can allow unsetting a password (set to null)', async () => {
+    const CustomizedMixin = objectionPassword({ allowEmptyPassword: true })
+    const CustomModel = generateCustomModel(CustomizedMixin)
+
+    const instance = await CustomModel.query().insert({ name: 'Dominic', password: 'hunter1' })
+    await instance.$query().patch({ password: null })
+
+    expect(instance.password).toBe(null)
+  })
+
+  it('can override the default password field', async () => {
+    // Use the name field instead of the password field as the password used by the plugin
+    const CustomizedMixin = objectionPassword({ passwordField: 'name' })
+    const CustomModel = generateCustomModel(CustomizedMixin)
+
+    const name = 'Dominic'
+    const password = 'hunter1'
+    const instance = await CustomModel.query().insert({ name, password })
+
+    expect(await instance.verifyPassword(password)).toBe(false)
+    expect(await instance.verifyPassword(name)).toBe(true)
+  })
+
+  it('can set the number of bcrypt hashing rounds', async () => {
+    // Expect to be called with 13 instead of 12
+    const CustomizedMixin = objectionPassword({ rounds: 13 })
+    const CustomModel = generateCustomModel(CustomizedMixin)
+
+    const bcryptSpy = jest.spyOn(bcrypt, 'hash')
+
+    const password = 'hunter1'
+    await CustomModel.query().insert({ name: 'Dominic', password })
+
+    expect(bcryptSpy).toHaveBeenCalledWith(password, 13)
+  })
+})
+
+describe('isBcryptHash', () => {
+  it('returns true when given a bcrypt hash', async () => {
+    expect(SampleModel.isBcryptHash(await bcrypt.hash('hello world', 12))).toBe(true)
+  })
+
+  it('returns false when given a regular string', () => {
+    expect(SampleModel.isBcryptHash('hello world')).toBe(false)
+  })
+})

jest.config.js

@@ -0,0 +1,13 @@
+// For a detailed explanation regarding each configuration property, visit:
+// https://jestjs.io/docs/en/configuration.html
+
+module.exports = {
+  // Automatically clear mock calls and instances between every test
+  clearMocks: true,
+
+  // The directory where Jest should output its coverage files
+  coverageDirectory: 'coverage',
+
+  // The test environment that will be used for testing
+  testEnvironment: 'node'
+}

package.json

@@ -1,13 +1,13 @@
 {
   "name": "objection-password",
-  "version": "2.1.0",
+  "version": "3.0.0",
   "description": "Automatic bcrypt hashing for Objection.js",
   "main": "index.js",
   "engines": {
     "node": "< 12.0.0"
   },
   "scripts": {
-    "test": "ava"
+    "test": "jest"
   },
   "repository": {
     "type": "git",
@@ -20,13 +20,17 @@
   },
   "homepage": "https://github.com/scoutforpets/objection-password#readme",
   "dependencies": {
-    "bcrypt": "^3.0.0"
+    "bcrypt": "^5.0.0"
   },
   "devDependencies": {
-    "ava": "^0.25.0",
-    "knex": "^0.14.4",
-    "objection": "^1.0.0",
-    "sqlite3": "^4.0.0",
-    "standard": "^12.0.1"
+    "jest": "^26.4.2",
+    "knex": "^0.21.5",
+    "objection": "^2.2.3",
+    "objection-transactional-tests": "^0.2.0",
+    "sqlite3": "^5.0.0",
+    "standard": "^14.3.4"
+  },
+  "engines": {
+    "node": ">= 12.0.0"
   }
 }