feat: advanced password validation using 'zxcvbn'

* skipFieldNames parameter
* additional tests

Author: pajgo

doc/password_validator.md

@@ -8,23 +8,92 @@ Validator dependencies:
 Validator configuration stored under `service.config.passwordValidator` key:
 
 ### minStrength _int_
-Sets minimal password complexity to accept a given password
+Sets minimal password complexity to accept a given password.
 
-### skipFieldName _string_
-Objects field name containing a boolean value. Allows skipping field validation if value === true Eg:
+### skipCheckFieldNames _string[]_
+Allows skipping field validation:
+
+- When passed field value is `boolean`:
+    * Validation skipped if `Object[field]` === `true`.
+    * Validation performed if `Object[field]` === `false`.
+- When passed field value is `any` type except `boolean`:
+    * Validation skipped if `Object[field]` set.
+    * Validation performed if `Object[field]` not exists.
+
+Eg:
 ```js
+const validatorConfig = {
+  minStrength: 4,
+  enabled: true,
+  skipCheckFieldNames: ['skipPassword'],
+};
+
+// Skip validation
+const dataSkipOnBool = {
+  myfield: 'fooBar', // schema "myfield":{"password": true} keyword
+  skipPassword: true,
+};
+
+// Skip validation `skipPassword` is string
+const dataSkipOnString = {
+  myfield: 'fooBar',
+  skipPassword: 'fooStringValue',
+};
+
+// Validation performed
+const dataSkipOnBoolFalse = {
+  myfield: 'fooBar',
+  skipPassword: false,
+};
 
 const data = {
-  myfield: 'fooBar', // in schema "myfield":{"password": true} keyword
-  validate: false,
+  myfield: 'fooBar',
+  skipPassword: true,
 };
 
+```
+
+### forceCheckFieldNames __string[]__
+Allows to force Validation event if the Validator disabled:
+- When any of passed fields value is `boolean`:
+    * Validation performed if `Object[field]` === `true`.
+    * Validation skipped if `Object[field]` === `false`.
+- When any of passed fields value is `any` type except `boolean`:
+    * Validation performed if `Object[field]` set.
+    * Validation skipped if `Object[field]` not exists.
+
+```js
 const validatorConfig = {
   minStrength: 4,
-  skipFieldName: 'validate', // if `data.validate` eq true 'data.myfield' is skipped during validation
+  enabled:false, // Will validate the `data` object anyway
+  forceCheckFieldNames: ['forceValidate'], // if `data.validate` not exists 'data.myfield' is skipped during validation
 }
+
+// Force validation
+const data = {
+  myfield: 'fooBar', // in schema "myfield":{"password": true} keyword.
+  forceValidate: true,
+};
+
+// Force validation, `forceValidate` is a string.
+const dataSkipOnString = {
+  myfield: 'fooBar',
+  forceValidate: 'fooStringValue',
+};
+
+// Validation NOT performed.
+const dataSkipOnBoolFalse = {
+  myfield: 'fooBar',
+  forceValidate: false,
+};
+
+// Validation NOT performed.
+const data = {
+  myfield: 'fooBar',
+};
 ```
-### inputFieldNames _array_
+
+### inputFieldNames _string[]_
 Allows using additional fields from the parent object. These values used inside `zxcvbn` to avoid sensitive information inside passwords.
 ```js
 const data = {
@@ -33,7 +102,7 @@ const data = {
   altname: 'barname',
 };
 
-// if 'username' and 'password' or other data will match, complexity level dropped
+// if 'username' and 'password' or other data will match, the complexity level dropped.
 const validatorConfig = {
   minStrength: 4,
   inputFieldNames: [

rfcs/password_validation_limits.md

@@ -19,6 +19,13 @@ A possible solution is to use existing password-strength checking solution like
 Enabling password validator is Breaking change and disabled by default.
 In the nearest future, when everything will be ready, a validator must be enabled.
 
+## OTP and Empty Password
+The `password` field is not defined as `required` in current `schemas`.
+Validation does not happen because This field does not exist in the OTP registration request.
+When the Service performs password generation,  `skipPassword` property does not exist in the registration request, so validation not executing on generated passwords.
+
+**NOTE:** Added additional test suites just to be sure.
+
 ## Validator Keyword
 Service validation algorithm based on the AJV validator and all validation requirements are inside schemas. All incoming requests checked. We can add additional custom validator `password` keyword for the `password` field, which performs required checks.
 
@@ -56,15 +63,20 @@ When executed:
 - calls `zxcvbn` to obtain a complexity of passwords and returns whether the given password matches policy
 
 Also, we can pass user-provided data into `zxcvbn` to check whether some sensitive data used in the password.
+
 ### Sample config
-`forceCheckFieldName` and `inputFieldNames` values point to parent object field
+`skipCheckFieldName` - Allows skipping password check if any field exists.
+`forceCheckFieldName` - Allows forcing password check if any field exists.
+`inputFieldNames` - values point to parent object fields that passed into `zxcvbn`.
+
 `enabled` - disable/enable validator. Default value is false.
 ```js
 const config = {
   enabled: false,
   minStrength: 3, // Desired strength
-  forceCheckFieldName: ['checkPassword'], // force enable password to check if the object field value set..
-  inputFieldNames: [ // Linked fields list, allow filter case sensitive data in the password from the parent object.
+  skipCheckFieldNames: ['skipPassword'], // Force disable password to check if the object field value exists.
+  forceCheckFieldNames: ['checkPassword'], // Force enable password check if the object field value exists.
+  inputFieldNames: [ // Linked fields list, allow filter the sensitive data in the password from the parent object.
     'username',
     'otherfield'
   ]

schemas/config.json

@@ -295,7 +295,15 @@
           "minimum": 0,
           "maximum": 4
         },
-        "forceCheckFieldName": {
+        "forceCheckFieldNames": {
+          "type": "array",
+          "items": {
+            "type": "string",
+            "minLength": 1
+          },
+          "default": []
+        },
+        "skipCheckFieldNames": {
           "type": "array",
           "items": {
             "type": "string",

src/configs/core.js

@@ -76,7 +76,8 @@ exports.logger = {
 exports.passwordValidator = {
   enabled: false,
   minStrength: 4, // 0..4
-  forceCheckFieldName: ['checkPassword'],
+  forceCheckFieldNames: ['checkPassword'],
+  skipCheckFieldNames: ['skipPassword'],
   inputFieldNames: [
     'username',
   ],

src/utils/passwordValidator.js

@@ -29,25 +29,37 @@ function validatorError(result, dataPath) {
   return { keyword: regKeyword, dataPath, message };
 }
 
+/**
+ * Checks whether any of the passed fields exist in the passed object
+ * @param object
+ * @param fields[string] list of fields
+ * @returns {boolean|*}
+ */
+function anyFieldExists(object, fields) {
+  if (Array.isArray(fields) && fields.length > 0) {
+    return fields.reduce((prev, fieldName) => {
+      if (prev) return prev;
+      if (object[fieldName]) return true;
+      return false;
+    }, false);
+  }
+  return false;
+}
+
 /**
  * Returns func for AJV validator keyword
  * @param {validatorConfig} config
  */
 function getValidatorFn(config) {
-  // service may not have its config changed after start
-  const { forceCheckFieldName, inputFieldNames, minStrength, enabled } = config;
+  const { forceCheckFieldNames, skipCheckFieldNames, inputFieldNames, minStrength, enabled } = config;
 
   return function validate(schema, data, parentSchema, currentPath, parentObject) {
-    let forceValidate;
-    if (Array.isArray(forceCheckFieldName) && forceCheckFieldName.length > 0) {
-      forceValidate = forceCheckFieldName.reduce((prev, fieldName) => {
-        if (prev) return prev;
-        if (parentObject[fieldName]) return true;
-        return false;
-      }, false);
-    }
+    // Force validation check
+    const forceValidate = anyFieldExists(parentObject, forceCheckFieldNames);
+    // Force skip validation check
+    const skipValidate = anyFieldExists(parentObject, skipCheckFieldNames);
 
-    const validatorEnabled = forceValidate || enabled;
+    const validatorEnabled = (forceValidate || enabled) && !skipValidate;
     if (!validatorEnabled) {
       return true;
     }

test/suites/register-stubbed.js

@@ -4,16 +4,16 @@ const is = require('is');
 const sinon = require('sinon').usingPromise(Promise);
 
 describe('#register stubbed', function suite() {
-  beforeEach(global.startService.bind(this));
-  afterEach(global.clearRedis.bind(this));
-
-  it('must be able to send activation code by sms', async () => {
+  /**
+   * Suite performing same checks but with different configuration contexts
+   */
+  const mustBeAbleToSendActivationCodeBySms = async () => {
     const amqpStub = sinon.stub(this.users.amqp, 'publishAndWait');
     const opts = {
       activate: false,
       audience: '*.localhost',
       challengeType: 'phone',
-      password: 'mynicepassword',
+      password: 'mynicepassword7521',
       username: '79215555555',
     };
 
@@ -42,9 +42,9 @@ describe('#register stubbed', function suite() {
     } finally {
       amqpStub.restore();
     }
-  });
+  };
 
-  it('must be able to send password by sms', async () => {
+  const mustBeAbleToSendPasswordBySms = async () => {
     const amqpStub = sinon.stub(this.users.amqp, 'publishAndWait');
     const opts = {
       activate: true,
@@ -76,9 +76,9 @@ describe('#register stubbed', function suite() {
     } finally {
       amqpStub.restore();
     }
-  });
+  };
 
-  it('should be able to register without password', async () => {
+  const shouldBeAbleToRegisterWithoutPassword = async () => {
     const amqpStub = sinon.stub(this.users.amqp, 'publishAndWait');
     const opts = {
       activate: false,
@@ -105,5 +105,27 @@ describe('#register stubbed', function suite() {
 
     const lastResponse = await this.dispatch('users.getInternalData', { username: '79215555555' });
     assert.equal(is.undefined(lastResponse.password), true);
+  };
+
+  describe('#password validator disabled', () => {
+    beforeEach(global.startService.bind(this));
+    afterEach(global.clearRedis.bind(this));
+
+    it('must be able to send activation code by sms', mustBeAbleToSendActivationCodeBySms);
+    it('must be able to send password by sms', mustBeAbleToSendPasswordBySms);
+    it('should be able to register without password', shouldBeAbleToRegisterWithoutPassword);
+  });
+
+  describe('#password validator enabled', () => {
+    beforeEach(async () => {
+      await global.startService.call(this, {
+        passwordValidator: { enabled: true },
+      });
+    });
+    afterEach(global.clearRedis.bind(this));
+
+    it('must be able to send activation code by sms', mustBeAbleToSendActivationCodeBySms);
+    it('must be able to send password by sms', mustBeAbleToSendPasswordBySms);
+    it('should be able to register without password', shouldBeAbleToRegisterWithoutPassword);
   });
 });

test/suites/register.js

@@ -257,6 +257,65 @@ describe('#register', function registerSuite() {
     });
   });
 
+  describe('password validator enabled', function testSuite() {
+    beforeEach(async function start() {
+      await global.startService.call(this, {
+        passwordValidator: { enabled: true },
+      });
+    });
+
+    afterEach(global.clearRedis);
+
+    it('must be able to create user without password validations and return user object and jwt token', function test() {
+      const opts = {
+        username: '[email protected]',
+        password: 'mynicepassword',
+        audience: 'matic.ninja',
+        skipPassword: true,
+        metadata: {
+          service: 'craft',
+        },
+      };
+
+      return this.dispatch('users.register', opts)
+        .then((registered) => {
+          assert(registered.hasOwnProperty('jwt'));
+          assert(registered.hasOwnProperty('user'));
+          assert.ok(registered.user.id);
+          assert(registered.user.hasOwnProperty('metadata'));
+          assert(registered.user.metadata.hasOwnProperty('matic.ninja'));
+          assert(registered.user.metadata.hasOwnProperty('*.localhost'));
+          assert.equal(registered.user.metadata['*.localhost'].username, opts.username);
+          assert.ifError(registered.user.password);
+          assert.ifError(registered.user.audience);
+        });
+    });
+
+    it('must be able to create user without validations and return user object and jwt token, password is auto-generated', function test() {
+      const opts = {
+        username: '[email protected]',
+        audience: 'matic.ninja',
+        metadata: {
+          service: 'craft',
+        },
+      };
+
+      return this.dispatch('users.register', opts)
+        .then((registered) => {
+          assert(registered.hasOwnProperty('jwt'));
+          assert(registered.hasOwnProperty('user'));
+          assert.ok(registered.user.id);
+          assert(registered.user.hasOwnProperty('metadata'));
+          assert(registered.user.metadata.hasOwnProperty('matic.ninja'));
+          assert(registered.user.metadata.hasOwnProperty('*.localhost'));
+          assert.equal(registered.user.metadata['*.localhost'].username, opts.username);
+          assert.ifError(registered.user.password);
+          assert.ifError(registered.user.audience);
+        });
+    });
+  });
+
+
   describe('must check password strength', function suite() {
     beforeEach(async function start() {
       await global.startService.call(this, {