Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for conditional bcrypt/bcryptjs usage #2523

Merged
merged 1 commit into from
Mar 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/tender-seahorses-attack.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@keystonejs/fields': major
---

Replaced default bcrypt implementation from `bcrypt` to `bcryptjs`. You can use the new `useCompiledBcrypt` config option to the `Password` field to keep the use of the `bcrypt` package. `bcrypt` must be manually listed in your `package.json` if use set `{ useCompiledBcrypt: true }`, as it is no longer a dependency of Keystone.
2 changes: 1 addition & 1 deletion docs/tutorials/add-lists.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ keystone.createList('User', UsersSchema);

<!-- FIXME:TL We haven't shown then how to get an Admin UI yes!!!! -->

Relaunch your app and check if new the list appeared in the Admin UI.
Relaunch your app and check if new the list appeared in the Admin UI.
But how can we assign a task to specific user? Let's proceed with [Defining Relationships](/docs/tutorials/relationships.md)

See also:
Expand Down
2 changes: 1 addition & 1 deletion packages/fields/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
"@keystonejs/utils": "^5.2.2",
"@sindresorhus/slugify": "^0.6.0",
"apollo-errors": "^1.9.0",
"bcrypt": "^3.0.6",
"bcryptjs": "^2.4.3",
"cuid": "^2.1.6",
"date-fns": "^1.30.1",
"dumb-passwords": "^0.2.1",
Expand Down
13 changes: 7 additions & 6 deletions packages/fields/src/types/Password/Implementation.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
import { Implementation } from '../../Implementation';
import { MongooseFieldAdapter } from '@keystonejs/adapter-mongoose';
import { KnexFieldAdapter } from '@keystonejs/adapter-knex';
import bcrypt from 'bcrypt';
import dumbPasswords from 'dumb-passwords';

const bcryptHashRegex = /^\$2[aby]?\$\d{1,2}\$[.\/A-Za-z0-9]{53}$/;

export class Password extends Implementation {
constructor(path, { rejectCommon, minLength, workFactor }) {
constructor(path, { rejectCommon, minLength, workFactor, useCompiledBcrypt }) {
super(...arguments);

this.bcrypt = require(useCompiledBcrypt ? 'bcrypt' : 'bcryptjs');

// Sanitise field specific config
this.rejectCommon = !!rejectCommon;
this.minLength = Math.max(Number.parseInt(minLength) || 8, 1);
Expand Down Expand Up @@ -50,18 +51,18 @@ export class Password extends Implementation {
// The compare() and compareSync() functions are constant-time
// The compare() and generateHash() functions will return a Promise if no call back is provided
compare(candidate, hash, callback) {
return bcrypt.compare(candidate, hash, callback);
return this.bcrypt.compare(candidate, hash, callback);
}
compareSync(candidate, hash) {
return bcrypt.compareSync(candidate, hash);
return this.bcrypt.compareSync(candidate, hash);
}
generateHash(plaintext, callback) {
this.validateNewPassword(plaintext);
return bcrypt.hash(plaintext, this.workFactor, callback);
return this.bcrypt.hash(plaintext, this.workFactor, callback);
}
generateHashSync(plaintext) {
this.validateNewPassword(plaintext);
return bcrypt.hashSync(plaintext, this.workFactor);
return this.bcrypt.hashSync(plaintext, this.workFactor);
}

extendAdminMeta(meta) {
Expand Down
24 changes: 18 additions & 6 deletions packages/fields/src/types/Password/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,13 @@ keystone.createList('User', {

### Config

| Option | Type | Default | Description |
| -------------- | --------- | ------- | --------------------------------------------------------------------- |
| `minLength` | `Integer` | `8` | The minimum number of characters this field will accept |
| `rejectCommon` | `Boolean` | `false` | Checks the password against a list of commonly used passwords |
| `workFactor` | `Integer` | `10` | Controls the processing time required to generate and validate hashes |
| `isRequired` | `Boolean` | `false` | Does this field require a value? |
| Option | Type | Default | Description |
| ------------------- | --------- | ------- | --------------------------------------------------------------------- |
| `minLength` | `Integer` | `8` | The minimum number of characters this field will accept |
| `rejectCommon` | `Boolean` | `false` | Checks the password against a list of commonly used passwords |
| `workFactor` | `Integer` | `10` | Controls the processing time required to generate and validate hashes |
| `isRequired` | `Boolean` | `false` | Does this field require a value? |
| `useCompiledBcrypt` | `Boolean` | `false` | Use the compiled `bcrypt` package rather than `bcryptjs` |

#### `minLength`

Expand Down Expand Up @@ -77,6 +78,17 @@ Note the `workFactor` supplied is applied by the bcrypt algorithm as an exponent
As such, a work factor of 11 will cause passwords to take _twice_ as long to hash and validate as a work factor of 10.
A work factor of 12 will cause passwords to take _four times_ as long as 10. Etc.

#### `useCompiledBcrypt`

By default the [`bcryptjs`](https://www.npmjs.com/package/bcryptjs) package is used for computing and comparing hashes.
This package provides a javascript implementation of the `bcrypt` algorithm.
A compiled version of this algorithm is provided by the [`bcrypt`](https://www.npmjs.com/package/bcrypt) package.
Setting `{ userCompiledBcrypt: true }` will tell Keystone to use the compiled package.
If you use this flag you must include `bcrypt` in your package dependencies.

The compiled package provides a ~20% performance improvement, and avoids the thread blocking of the JavaScript implementation.
This comes with the trade off that the compiled package can be challenging to work with when working in some environments (e.g. Windows) or when trying to deploy code built in one environment onto a different environment (e.g. build on OSX to deploy in a Linux based lambda process).

### Auth Strategies

The `Password` field exposes a `compare()` function.
Expand Down
18 changes: 5 additions & 13 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5269,13 +5269,10 @@ bcrypt-pbkdf@^1.0.0:
dependencies:
tweetnacl "^0.14.3"

bcrypt@^3.0.6:
version "3.0.6"
resolved "https://registry.yarnpkg.com/bcrypt/-/bcrypt-3.0.6.tgz#f607846df62d27e60d5e795612c4f67d70206eb2"
integrity sha512-taA5bCTfXe7FUjKroKky9EXpdhkVvhE5owfxfLYodbrAR1Ul3juLmIQmIQBK4L9a5BuUcE6cqmwT+Da20lF9tg==
dependencies:
nan "2.13.2"
node-pre-gyp "0.12.0"
bcryptjs@^2.4.3:
version "2.4.3"
resolved "https://registry.yarnpkg.com/bcryptjs/-/bcryptjs-2.4.3.tgz#9ab5627b93e60621ff7cdac5da9733027df1d0cb"
integrity sha1-mrVie5PmBiH/fNrF2pczAn3x0Ms=

better-assert@~1.0.0:
version "1.0.2"
Expand Down Expand Up @@ -16023,11 +16020,6 @@ name-all-modules-plugin@^1.0.1:
resolved "https://registry.yarnpkg.com/name-all-modules-plugin/-/name-all-modules-plugin-1.0.1.tgz#0abfb6ad835718b9fb4def0674e06657a954375c"
integrity sha1-Cr+2rYNXGLn7Te8GdOBmV6lUN1w=

[email protected]:
version "2.13.2"
resolved "https://registry.yarnpkg.com/nan/-/nan-2.13.2.tgz#f51dc7ae66ba7d5d55e1e6d4d8092e802c9aefe7"
integrity sha512-TghvYc72wlMGMVMluVo9WRJc0mB8KxxF/gZ4YYFy7V2ZQX9l7rgbPg7vjS9mt6U5HXODVFVI2bOduCzwOMv/lw==

nan@^2.12.1, nan@^2.14.0:
version "2.14.0"
resolved "https://registry.yarnpkg.com/nan/-/nan-2.14.0.tgz#7818f722027b2459a86f0295d434d1fc2336c52c"
Expand Down Expand Up @@ -16323,7 +16315,7 @@ node-object-hash@^2.0.0:
resolved "https://registry.yarnpkg.com/node-object-hash/-/node-object-hash-2.0.0.tgz#9971fcdb7d254f05016bd9ccf508352bee11116b"
integrity sha512-VZR0zroAusy1ETZMZiGeLkdu50LGjG5U1KHZqTruqtTyQ2wfWhHG2Ow4nsUbfTFGlaREgNHcCWoM/OzEm6p+NQ==

node-pre-gyp@0.12.0, node-pre-gyp@^0.12.0:
node-pre-gyp@^0.12.0:
version "0.12.0"
resolved "https://registry.yarnpkg.com/node-pre-gyp/-/node-pre-gyp-0.12.0.tgz#39ba4bb1439da030295f899e3b520b7785766149"
integrity sha512-4KghwV8vH5k+g2ylT+sLTjy5wmUOb9vPhnM8NHvRf9dHmnW/CndrFXy2aRPaPST6dugXSdHXfeaHQm77PIz/1A==
Expand Down