Migrate to bcrypt (#8446)

* start migrating to bcrypt

* added method to convert the password to bcrypt when logging in, added method to compare password without knowing the hashing algorhytm, remove default

* travis: try to upgrade to container based infrastructure

* travis: add deps to build bcrypt.js

* travis: add deps to build bcrypt.js

* travis: add deps to build bcrypt.js

* travis: add deps to build bcrypt.js

* use bcryptjs until bcrypt can be installed on travis, see https://github.com/kelektiv/node.bcrypt.js/issues/476

* correct sha1 unit tests

* try different mongodb repo

* try without mognodb services

* try again with bcrypt

* disable request logging in travis

* migrate missing routes

* simplify code

* remove bcryptjs

* fix typo

* fix typo

* fix typo in comment

* add unit tests for new passwords utility emthods

* travis: back to old infrastructure, containers often have timeouts

* add integration test for passwordHashMethod

* update shrinkwrap

* clarify code and add comments

* add integration tests

* fix linting

* fix integration tests

.travis.yml

@@ -1,7 +1,15 @@
 language: node_js
 node_js:
   - '6'
+sudo: required
+addons:
+  apt:
+    sources:
+      - ubuntu-toolchain-r-test
+    packages:
+      - g++-4.8
 before_install:
+  - $CXX --version
   - npm install -g npm@4
   - if [ $REQUIRES_SERVER ]; then sudo apt-key adv --keyserver hkp://keyserver.ubuntu.com:80 --recv 7F0CEB10; echo 'deb http://downloads-distro.mongodb.org/repo/ubuntu-upstart dist 10gen' | sudo tee /etc/apt/sources.list.d/mongodb.list; sudo apt-get update; sudo apt-get install mongodb-org-server; fi
 before_script:
@@ -12,6 +20,9 @@ after_script:
   - ./node_modules/.bin/lcov-result-merger 'coverage/**/*.info' | ./node_modules/coveralls/bin/coveralls.js
 script: npm run $TEST
 env:
+  global:
+    - CXX=g++-4.8
+    - DISABLE_REQUEST_LOGGING=true
   matrix:
     - TEST="lint"
     - TEST="test:api-v3" REQUIRES_SERVER=true

package.json

@@ -25,6 +25,7 @@
     "babel-register": "^6.6.0",
     "babel-runtime": "^6.11.6",
     "babelify": "^7.2.0",
+    "bcrypt": "^1.0.2",
     "bluebird": "^3.3.5",
     "body-parser": "^1.15.0",
     "bower": "~1.3.12",

test/api/v3/integration/user/DELETE-user.test.js

@@ -11,6 +11,10 @@ import {
   map,
 } from 'lodash';
 import Bluebird from 'bluebird';
+import {
+  sha1MakeSalt,
+  sha1Encrypt as sha1EncryptPassword,
+} from '../../../../../website/server/libs/password';
 
 describe('DELETE /user', () => {
   let user;
@@ -67,6 +71,30 @@ describe('DELETE /user', () => {
     await expect(checkExistence('users', user._id)).to.eventually.eql(false);
   });
 
+  it('deletes the user with a legacy sha1 password', async () => {
+    let textPassword = 'mySecretPassword';
+    let salt = sha1MakeSalt();
+    let sha1HashedPassword = sha1EncryptPassword(textPassword, salt);
+
+    await user.update({
+      'auth.local.hashed_password': sha1HashedPassword,
+      'auth.local.passwordHashMethod': 'sha1',
+      'auth.local.salt': salt,
+    });
+
+    await user.sync();
+
+    expect(user.auth.local.passwordHashMethod).to.equal('sha1');
+    expect(user.auth.local.salt).to.equal(salt);
+    expect(user.auth.local.hashed_password).to.equal(sha1HashedPassword);
+
+    // delete the user
+    await user.del('/user', {
+      password: textPassword,
+    });
+    await expect(checkExistence('users', user._id)).to.eventually.eql(false);
+  });
+
   context('last member of a party', () => {
     let party;
 

test/api/v3/integration/user/GET-user.test.js

@@ -23,6 +23,7 @@ describe('GET /user', () => {
     let returnedUser = await user.get('/user');
 
     expect(returnedUser.auth.local.hashed_password).to.not.exist;
+    expect(returnedUser.auth.local.passwordHashMethod).to.not.exist;
     expect(returnedUser.auth.local.salt).to.not.exist;
     expect(returnedUser.apiToken).to.not.exist;
   });

test/api/v3/integration/user/auth/POST-login-local.test.js

@@ -3,6 +3,11 @@ import {
   requester,
   translate as t,
 } from '../../../../../helpers/api-integration/v3';
+import {
+  bcryptCompare,
+  sha1MakeSalt,
+  sha1Encrypt as sha1EncryptPassword,
+} from '../../../../../../website/server/libs/password';
 
 describe('POST /user/auth/local/login', () => {
   let api;
@@ -72,4 +77,35 @@ describe('POST /user/auth/local/login', () => {
       message: t('invalidReqParams'),
     });
   });
+
+  it('converts user with SHA1 encrypted password to bcrypt encryption', async () => {
+    let textPassword = 'mySecretPassword';
+    let salt = sha1MakeSalt();
+    let sha1HashedPassword = sha1EncryptPassword(textPassword, salt);
+
+    await user.update({
+      'auth.local.hashed_password': sha1HashedPassword,
+      'auth.local.passwordHashMethod': 'sha1',
+      'auth.local.salt': salt,
+    });
+
+    await user.sync();
+    expect(user.auth.local.passwordHashMethod).to.equal('sha1');
+    expect(user.auth.local.salt).to.equal(salt);
+    expect(user.auth.local.hashed_password).to.equal(sha1HashedPassword);
+
+    // login
+    await api.post(endpoint, {
+      username: user.auth.local.email,
+      password: textPassword,
+    });
+
+    await user.sync();
+    expect(user.auth.local.passwordHashMethod).to.equal('bcrypt');
+    expect(user.auth.local.salt).to.be.undefined;
+    expect(user.auth.local.hashed_password).not.to.equal(sha1HashedPassword);
+
+    let isValidPassword = await bcryptCompare(textPassword, user.auth.local.hashed_password);
+    expect(isValidPassword).to.equal(true);
+  });
 });

test/api/v3/integration/user/auth/POST-user_reset_password.test.js

@@ -2,6 +2,10 @@ import {
   generateUser,
   translate as t,
 } from '../../../../../helpers/api-integration/v3';
+import {
+  sha1MakeSalt,
+  sha1Encrypt as sha1EncryptPassword,
+} from '../../../../../../website/server/libs/password';
 
 describe('POST /user/reset-password', async () => {
   let endpoint = '/user/reset-password';
@@ -21,6 +25,33 @@ describe('POST /user/reset-password', async () => {
     expect(user.auth.local.hashed_password).to.not.eql(previousPassword);
   });
 
+  it('converts user with SHA1 encrypted password to bcrypt encryption', async () => {
+    let textPassword = 'mySecretPassword';
+    let salt = sha1MakeSalt();
+    let sha1HashedPassword = sha1EncryptPassword(textPassword, salt);
+
+    await user.update({
+      'auth.local.hashed_password': sha1HashedPassword,
+      'auth.local.passwordHashMethod': 'sha1',
+      'auth.local.salt': salt,
+    });
+
+    await user.sync();
+    expect(user.auth.local.passwordHashMethod).to.equal('sha1');
+    expect(user.auth.local.salt).to.equal(salt);
+    expect(user.auth.local.hashed_password).to.equal(sha1HashedPassword);
+
+    // update email
+    await user.post(endpoint, {
+      email: user.auth.local.email,
+    });
+
+    await user.sync();
+    expect(user.auth.local.passwordHashMethod).to.equal('bcrypt');
+    expect(user.auth.local.salt).to.be.undefined;
+    expect(user.auth.local.hashed_password).not.to.equal(sha1HashedPassword);
+  });
+
   it('same message on error as on success', async () => {
     let response = await user.post(endpoint, {
       email: 'nonExistent@email.com',

test/api/v3/integration/user/auth/PUT-user_update_email.test.js

@@ -2,6 +2,11 @@ import {
   generateUser,
   translate as t,
 } from '../../../../../helpers/api-v3-integration.helper';
+import {
+  bcryptCompare,
+  sha1MakeSalt,
+  sha1Encrypt as sha1EncryptPassword,
+} from '../../../../../../website/server/libs/password';
 
 const ENDPOINT = '/user/auth/update-email';
 
@@ -66,6 +71,41 @@ describe('PUT /user/auth/update-email', () => {
         message: t('cannotFulfillReq'),
       });
     });
+
+    it('converts user with SHA1 encrypted password to bcrypt encryption', async () => {
+      let textPassword = 'mySecretPassword';
+      let salt = sha1MakeSalt();
+      let sha1HashedPassword = sha1EncryptPassword(textPassword, salt);
+      let myNewEmail = 'my-new-random-email@example.net';
+
+      await user.update({
+        'auth.local.hashed_password': sha1HashedPassword,
+        'auth.local.passwordHashMethod': 'sha1',
+        'auth.local.salt': salt,
+      });
+
+      await user.sync();
+      expect(user.auth.local.passwordHashMethod).to.equal('sha1');
+      expect(user.auth.local.salt).to.equal(salt);
+      expect(user.auth.local.hashed_password).to.equal(sha1HashedPassword);
+
+      // update email
+      let response = await user.put(ENDPOINT, {
+        newEmail: myNewEmail,
+        password: textPassword,
+      });
+      expect(response).to.eql({ email: myNewEmail });
+
+      await user.sync();
+
+      expect(user.auth.local.email).to.equal(myNewEmail);
+      expect(user.auth.local.passwordHashMethod).to.equal('bcrypt');
+      expect(user.auth.local.salt).to.be.undefined;
+      expect(user.auth.local.hashed_password).not.to.equal(sha1HashedPassword);
+
+      let isValidPassword = await bcryptCompare(textPassword, user.auth.local.hashed_password);
+      expect(isValidPassword).to.equal(true);
+    });
   });
 
   context('Social Login User', async () => {

test/api/v3/integration/user/auth/PUT-user_update_password.test.js

@@ -2,6 +2,11 @@ import {
   generateUser,
   translate as t,
 } from '../../../../../helpers/api-v3-integration.helper';
+import {
+  bcryptCompare,
+  sha1MakeSalt,
+  sha1Encrypt as sha1EncryptPassword,
+} from '../../../../../../website/server/libs/password';
 
 const ENDPOINT = '/user/auth/update-password';
 
@@ -89,4 +94,36 @@ describe('PUT /user/auth/update-password', async () => {
       message: t('invalidReqParams'),
     });
   });
+
+  it('converts user with SHA1 encrypted password to bcrypt encryption', async () => {
+    let textPassword = 'mySecretPassword';
+    let salt = sha1MakeSalt();
+    let sha1HashedPassword = sha1EncryptPassword(textPassword, salt);
+
+    await user.update({
+      'auth.local.hashed_password': sha1HashedPassword,
+      'auth.local.passwordHashMethod': 'sha1',
+      'auth.local.salt': salt,
+    });
+
+    user.sync();
+    expect(user.auth.local.passwordHashMethod).to.equal('sha1');
+    expect(user.auth.local.salt).to.equal(salt);
+    expect(user.auth.local.hashed_password).to.equal(sha1HashedPassword);
+
+    // update email
+    await user.put(ENDPOINT, {
+      password: textPassword,
+      newPassword,
+      confirmPassword: newPassword,
+    });
+
+    await user.sync();
+    expect(user.auth.local.passwordHashMethod).to.equal('bcrypt');
+    expect(user.auth.local.salt).to.be.undefined;
+    expect(user.auth.local.hashed_password).not.to.equal(sha1HashedPassword);
+
+    let isValidPassword = await bcryptCompare(newPassword, user.auth.local.hashed_password);
+    expect(isValidPassword).to.equal(true);
+  });
 });

test/api/v3/integration/user/auth/PUT-user_update_username.test.js

@@ -2,6 +2,11 @@ import {
   generateUser,
   translate as t,
 } from '../../../../../helpers/api-v3-integration.helper';
+import {
+  bcryptCompare,
+  sha1MakeSalt,
+  sha1Encrypt as sha1EncryptPassword,
+} from '../../../../../../website/server/libs/password';
 
 const ENDPOINT = '/user/auth/update-username';
 
@@ -24,6 +29,41 @@ describe('PUT /user/auth/update-username', async () => {
     expect(user.auth.local.username).to.eql(newUsername);
   });
 
+  it('converts user with SHA1 encrypted password to bcrypt encryption', async () => {
+    let myNewUsername = 'my-new-username';
+    let textPassword = 'mySecretPassword';
+    let salt = sha1MakeSalt();
+    let sha1HashedPassword = sha1EncryptPassword(textPassword, salt);
+
+    await user.update({
+      'auth.local.hashed_password': sha1HashedPassword,
+      'auth.local.passwordHashMethod': 'sha1',
+      'auth.local.salt': salt,
+    });
+
+    await user.sync();
+    expect(user.auth.local.passwordHashMethod).to.equal('sha1');
+    expect(user.auth.local.salt).to.equal(salt);
+    expect(user.auth.local.hashed_password).to.equal(sha1HashedPassword);
+
+    // update email
+    let response = await user.put(ENDPOINT, {
+      username: myNewUsername,
+      password: textPassword,
+    });
+    expect(response).to.eql({ username: myNewUsername });
+
+    await user.sync();
+
+    expect(user.auth.local.username).to.eql(myNewUsername);
+    expect(user.auth.local.passwordHashMethod).to.equal('bcrypt');
+    expect(user.auth.local.salt).to.be.undefined;
+    expect(user.auth.local.hashed_password).not.to.equal(sha1HashedPassword);
+
+    let isValidPassword = await bcryptCompare(textPassword, user.auth.local.hashed_password);
+    expect(isValidPassword).to.equal(true);
+  });
+
   context('errors', async () => {
     it('prevents username update if new username is already taken', async () => {
       let existingUsername = 'existing-username';

test/api/v3/unit/libs/password.test.js

@@ -1,41 +1,240 @@
+/* eslint-disable camelcase */
+
 import {
-  encrypt as encryptPassword,
-  makeSalt,
+  sha1Encrypt as sha1EncryptPassword,
+  sha1MakeSalt,
+  bcryptHash,
+  bcryptCompare,
+  compare,
+  convertToBcrypt,
 } from '../../../../../website/server/libs/password';
 
 describe('Password Utilities', () => {
-  describe('Encrypt', () => {
-    it('always encrypt the same password to the same value when using the same salt', () => {
+  describe('compare', () => {
+    it('can compare a correct password hashed with SHA1', async () => {
       let textPassword = 'mySecretPassword';
-      let salt = makeSalt();
-      let encryptedPassword = encryptPassword(textPassword, salt);
+      let salt = sha1MakeSalt();
+      let hashedPassword = sha1EncryptPassword(textPassword, salt);
 
-      expect(encryptPassword(textPassword, salt)).to.eql(encryptedPassword);
+      let user = {
+        auth: {
+          local: {
+            hashed_password: hashedPassword,
+            salt,
+            passwordHashMethod: 'sha1',
+          },
+        },
+      };
+
+      let isValidPassword = await compare(user, textPassword);
+      expect(isValidPassword).to.eql(true);
     });
 
-    it('never encrypt the same password to the same value when using a different salt', () => {
+    it('can compare an invalid password hashed with SHA1', async () => {
       let textPassword = 'mySecretPassword';
-      let aSalt = makeSalt();
-      let anotherSalt = makeSalt();
-      let anEncryptedPassword = encryptPassword(textPassword, aSalt);
-      let anotherEncryptedPassword = encryptPassword(textPassword, anotherSalt);
+      let salt = sha1MakeSalt();
+      let hashedPassword = sha1EncryptPassword(textPassword, salt);
 
-      expect(anEncryptedPassword).not.to.eql(anotherEncryptedPassword);
+      let user = {
+        auth: {
+          local: {
+            hashed_password: hashedPassword,
+            salt,
+            passwordHashMethod: 'sha1',
+          },
+        },
+      };
+
+      let isValidPassword = await compare(user, 'wrongPassword');
+      expect(isValidPassword).to.eql(false);
+    });
+
+    it('can compare a correct password hashed with bcrypt', async () => {
+      let textPassword = 'mySecretPassword';
+      let hashedPassword = await bcryptHash(textPassword);
+
+      let user = {
+        auth: {
+          local: {
+            hashed_password: hashedPassword,
+            passwordHashMethod: 'bcrypt',
+          },
+        },
+      };
+
+      let isValidPassword = await compare(user, textPassword);
+      expect(isValidPassword).to.eql(true);
+    });
+
+    it('can compare an invalid password hashed with bcrypt', async () => {
+      let textPassword = 'mySecretPassword';
+      let hashedPassword = await bcryptHash(textPassword);
+
+      let user = {
+        auth: {
+          local: {
+            hashed_password: hashedPassword,
+            passwordHashMethod: 'bcrypt',
+          },
+        },
+      };
+
+      let isValidPassword = await compare(user, 'wrongPassword');
+      expect(isValidPassword).to.eql(false);
+    });
+
+    it('throws an error if user is missing', async () => {
+      try {
+        await compare(null, 'some password');
+      } catch (e) {
+        expect(e.toString()).to.equal('Error: user and passwordToCheck are required parameters.');
+      }
+    });
+
+    it('throws an error if passwordToCheck is missing', async () => {
+      try {
+        await compare({a: true});
+      } catch (e) {
+        expect(e.toString()).to.equal('Error: user and passwordToCheck are required parameters.');
+      }
+    });
+
+    it('throws an error if an invalid hashing method is used', async () => {
+      try {
+        await compare({
+          auth: {
+            local: {
+              passwordHashMethod: 'invalid',
+            },
+          },
+        }, 'pass');
+      } catch (e) {
+        expect(e.toString()).to.equal('Error: Invalid password hash method.');
+      }
+    });
+
+    it('returns true if comparing the same password', async () => {
+      let textPassword = 'mySecretPassword';
+      let hashedPassword = await bcryptHash(textPassword);
+
+      let isValidPassword = await bcryptCompare(textPassword, hashedPassword);
+      expect(isValidPassword).to.eql(true);
+    });
+
+    it('returns true if comparing a different password', async () => {
+      let textPassword = 'mySecretPassword';
+      let hashedPassword = await bcryptHash(textPassword);
+
+      let isValidPassword = await bcryptCompare('anotherPassword', hashedPassword);
+      expect(isValidPassword).to.eql(false);
     });
   });
 
-  describe('Make Salt', () => {
-    it('creates a salt with length 10 by default', () => {
-      let salt = makeSalt();
+  describe('convertToBcrypt', () => {
+    it('converts an user password hashed with sha1 to bcrypt', async () => {
+      let textPassword = 'mySecretPassword';
+      let salt = sha1MakeSalt();
+      let hashedPassword = sha1EncryptPassword(textPassword, salt);
 
-      expect(salt.length).to.eql(10);
+      let user = {
+        auth: {
+          local: {
+            hashed_password: hashedPassword,
+            salt,
+            passwordHashMethod: 'sha1',
+          },
+        },
+      };
+
+      await convertToBcrypt(user, textPassword);
+      expect(user.auth.local.salt).to.be.undefined;
+      expect(user.auth.local.passwordHashMethod).to.equal('bcrypt');
+      expect(user.auth.local.hashed_password).to.be.a.string;
+
+      let isValidPassword = await compare(user, textPassword);
+      expect(isValidPassword).to.eql(true);
     });
 
-    it('can create a salt of any length', () => {
-      let length = 24;
-      let salt = makeSalt(length);
+    it('throws an error if user is missing', async () => {
+      try {
+        await convertToBcrypt(null, 'string');
+      } catch (e) {
+        expect(e.toString()).to.equal('Error: user and plainTextPassword are required parameters.');
+      }
+    });
 
-      expect(salt.length).to.eql(length);
+    it('throws an error if plainTextPassword is missing', async () => {
+      try {
+        await convertToBcrypt({a: true});
+      } catch (e) {
+        expect(e.toString()).to.equal('Error: user and plainTextPassword are required parameters.');
+      }
+    });
+  });
+
+  describe('bcrypt', () => {
+    describe('Hash', () => {
+      it('returns a hashed string', async () => {
+        let textPassword = 'mySecretPassword';
+        let hashedPassword = await bcryptHash(textPassword);
+
+        expect(hashedPassword).to.be.a.string;
+      });
+    });
+
+    describe('Compare', () => {
+      it('returns true if comparing the same password', async () => {
+        let textPassword = 'mySecretPassword';
+        let hashedPassword = await bcryptHash(textPassword);
+
+        let isValidPassword = await bcryptCompare(textPassword, hashedPassword);
+        expect(isValidPassword).to.eql(true);
+      });
+
+      it('returns true if comparing a different password', async () => {
+        let textPassword = 'mySecretPassword';
+        let hashedPassword = await bcryptHash(textPassword);
+
+        let isValidPassword = await bcryptCompare('anotherPassword', hashedPassword);
+        expect(isValidPassword).to.eql(false);
+      });
+    });
+  });
+
+  describe('SHA1', () => {
+    describe('Encrypt', () => {
+      it('always encrypt the same password to the same value when using the same salt', () => {
+        let textPassword = 'mySecretPassword';
+        let salt = sha1MakeSalt();
+        let encryptedPassword = sha1EncryptPassword(textPassword, salt);
+
+        expect(sha1EncryptPassword(textPassword, salt)).to.eql(encryptedPassword);
+      });
+
+      it('never encrypt the same password to the same value when using a different salt', () => {
+        let textPassword = 'mySecretPassword';
+        let aSalt = sha1MakeSalt();
+        let anotherSalt = sha1MakeSalt();
+        let anEncryptedPassword = sha1EncryptPassword(textPassword, aSalt);
+        let anotherEncryptedPassword = sha1EncryptPassword(textPassword, anotherSalt);
+
+        expect(anEncryptedPassword).not.to.eql(anotherEncryptedPassword);
+      });
+    });
+
+    describe('Make Salt', () => {
+      it('creates a salt with length 10 by default', () => {
+        let salt = sha1MakeSalt();
+
+        expect(salt.length).to.eql(10);
+      });
+
+      it('can create a salt of any length', () => {
+        let length = 24;
+        let salt = sha1MakeSalt(length);
+
+        expect(salt.length).to.eql(length);
+      });
     });
   });
 });

test/helpers/mongo.js

@@ -55,8 +55,8 @@ export async function resetHabiticaDB () {
               username: 'username',
               lowerCaseUsername: 'username',
               email: 'username@email.com',
-              salt: 'salt',
               hashed_password: 'hashed_password', // eslint-disable-line camelcase
+              passwordHashMethod: 'bcrypt',
             },
           },
         }, (insertErr) => {

website/server/controllers/api-v3/auth.js

@@ -115,17 +115,15 @@ api.registerLocal = {
       if (lowerCaseUsername === user.auth.local.lowerCaseUsername) throw new NotAuthorized(res.t('usernameTaken'));
     }
 
-    let salt = passwordUtils.makeSalt();
-    let hashed_password = passwordUtils.encrypt(password, salt); // eslint-disable-line camelcase
+    let hashed_password = await passwordUtils.bcryptHash(password); // eslint-disable-line camelcase
     let newUser = {
       auth: {
         local: {
           username,
           lowerCaseUsername,
           email,
-          salt,
-          hashed_password, // eslint-disable-line camelcase
-          passwordHashMethod: 'sha1',
+          hashed_password, // eslint-disable-line camelcase,
+          passwordHashMethod: 'bcrypt',
         },
       },
       preferences: {
@@ -223,6 +221,7 @@ api.loginLocal = {
 
     let login;
     let username = req.body.username;
+    let password = req.body.password;
 
     if (validator.isEmail(username)) {
       login = {'auth.local.email': username.toLowerCase()}; // Emails are stored lowercase
@@ -230,10 +229,25 @@ api.loginLocal = {
       login = {'auth.local.username': username};
     }
 
-    let user = await User.findOne(login, {auth: 1, apiToken: 1}).exec();
-    let isValidPassword = user && user.auth.local.hashed_password === passwordUtils.encrypt(req.body.password, user.auth.local.salt);
+    // load the entire user because we may have to save it to convert the password to bcrypt
+    let user = await User.findOne(login).exec();
+
+    let isValidPassword;
+
+    if (!user) {
+      isValidPassword = false;
+    } else {
+      isValidPassword = await passwordUtils.compare(user, password);
+    }
+
     if (!isValidPassword) throw new NotAuthorized(res.t('invalidLoginCredentialsLong'));
 
+    // convert the hashed password to bcrypt from sha1
+    if (user.auth.local.passwordHashMethod === 'sha1') {
+      await passwordUtils.convertToBcrypt(user, password);
+      await user.save();
+    }
+
     res.analytics.track('login', {
       category: 'behaviour',
       type: 'local',
@@ -433,12 +447,18 @@ api.updateUsername = {
 
     if (!user.auth.local.username) throw new BadRequest(res.t('userHasNoLocalRegistration'));
 
-    let oldPassword = passwordUtils.encrypt(req.body.password, user.auth.local.salt);
-    if (oldPassword !== user.auth.local.hashed_password) throw new NotAuthorized(res.t('wrongPassword'));
+    let password = req.body.password;
+    let isValidPassword = await passwordUtils.compare(user, password);
+    if (!isValidPassword) throw new NotAuthorized(res.t('wrongPassword'));
 
     let count = await User.count({ 'auth.local.lowerCaseUsername': req.body.username.toLowerCase() });
     if (count > 0) throw new BadRequest(res.t('usernameTaken'));
 
+    // if password is using old sha1 encryption, change it
+    if (user.auth.local.passwordHashMethod === 'sha1') {
+      await passwordUtils.convertToBcrypt(user, password); // user is saved a few lines below
+    }
+
     // save username
     user.auth.local.lowerCaseUsername = req.body.username.toLowerCase();
     user.auth.local.username = req.body.username;
@@ -487,13 +507,17 @@ api.updatePassword = {
       throw validationErrors;
     }
 
-    let oldPassword = passwordUtils.encrypt(req.body.password, user.auth.local.salt);
-    if (oldPassword !== user.auth.local.hashed_password) throw new NotAuthorized(res.t('wrongPassword'));
+    let oldPassword = req.body.password;
+    let isValidPassword = await passwordUtils.compare(user, oldPassword);
+    if (!isValidPassword) throw new NotAuthorized(res.t('wrongPassword'));
 
-    if (req.body.newPassword !== req.body.confirmPassword) throw new NotAuthorized(res.t('passwordConfirmationMatch'));
+    let newPassword = req.body.newPassword;
+    if (newPassword !== req.body.confirmPassword) throw new NotAuthorized(res.t('passwordConfirmationMatch'));
 
-    user.auth.local.hashed_password = passwordUtils.encrypt(req.body.newPassword, user.auth.local.salt); // eslint-disable-line camelcase
+    // set new password and make sure it's using bcrypt for hashing
+    await passwordUtils.convertToBcrypt(user, newPassword);
     await user.save();
+
     res.respond(200, {});
   },
 };
@@ -522,15 +546,15 @@ api.resetPassword = {
     if (validationErrors) throw validationErrors;
 
     let email = req.body.email.toLowerCase();
-    let salt = passwordUtils.makeSalt();
-    let newPassword =  passwordUtils.makeSalt(); // use a salt as the new password too (they'll change it later)
-    let hashedPassword = passwordUtils.encrypt(newPassword, salt);
-
     let user = await User.findOne({ 'auth.local.email': email }).exec();
 
     if (user) {
-      user.auth.local.salt = salt;
-      user.auth.local.hashed_password = hashedPassword; // eslint-disable-line camelcase
+      // use a salt as the new password too (they'll change it later)
+      let newPassword =  passwordUtils.sha1MakeSalt();
+
+      // set new password and make sure it's using bcrypt for hashing
+      await passwordUtils.convertToBcrypt(user, newPassword); // user is saved a few lines below
+
       sendEmail({
         from: 'Habitica <admin@habitica.com>',
         to: email,
@@ -585,8 +609,14 @@ api.updateEmail = {
 
     if (emailAlreadyInUse) throw new NotAuthorized(res.t('cannotFulfillReq'));
 
-    let candidatePassword = passwordUtils.encrypt(req.body.password, user.auth.local.salt);
-    if (candidatePassword !== user.auth.local.hashed_password) throw new NotAuthorized(res.t('wrongPassword'));
+    let password = req.body.password;
+    let isValidPassword = await passwordUtils.compare(user, password);
+    if (!isValidPassword) throw new NotAuthorized(res.t('wrongPassword'));
+
+    // if password is using old sha1 encryption, change it
+    if (user.auth.local.passwordHashMethod === 'sha1') {
+      await passwordUtils.convertToBcrypt(user, password);
+    }
 
     user.auth.local.email = req.body.newEmail;
     await user.save();

website/server/controllers/api-v3/user.js

@@ -200,8 +200,9 @@ api.deleteUser = {
     let validationErrors = req.validationErrors();
     if (validationErrors) throw validationErrors;
 
-    let oldPassword = passwordUtils.encrypt(req.body.password, user.auth.local.salt);
-    if (oldPassword !== user.auth.local.hashed_password) throw new NotAuthorized(res.t('wrongPassword'));
+    let password = req.body.password;
+    let isValidPassword = await passwordUtils.compare(user, password);
+    if (!isValidPassword) throw new NotAuthorized(res.t('wrongPassword'));
 
     if (plan && plan.customerId && !plan.dateTerminated) {
       throw new NotAuthorized(res.t('cannotDeleteActiveAccount'));
@@ -256,6 +257,7 @@ api.getUserAnonymized = {
     if (user.auth) {
       delete user.auth.local;
       delete user.auth.facebook;
+      delete user.auth.google;
     }
     delete user.newMessages;
     delete user.profile;

website/server/libs/password.js

@@ -1,8 +1,22 @@
 // Utilities for working with passwords
 import crypto from 'crypto';
+import bcrypt from 'bcrypt';
+
+const BCRYPT_SALT_ROUNDS = 10;
+
+// Hash a plain text password
+export function bcryptHash (passwordToHash) {
+  return bcrypt.hash(passwordToHash, BCRYPT_SALT_ROUNDS); // returns a promise
+}
+
+// Check if a plain text password matches a hash
+export function bcryptCompare (passwordToCheck, hashedPassword) {
+  return bcrypt.compare(passwordToCheck, hashedPassword); // returns a promise
+}
 
 // Return the encrypted version of a password (using sha1) given a salt
-export function encrypt (password, salt) {
+// Used for legacy passwords that have not yet been migrated to bcrypt
+export function sha1Encrypt (password, salt) {
   return crypto
           .createHmac('sha1', salt)
           .update(password)
@@ -10,9 +24,42 @@ export function encrypt (password, salt) {
 }
 
 // Create a salt, default length is 10
-export function makeSalt (len = 10) {
+export function sha1MakeSalt (len = 10) {
   return crypto
           .randomBytes(Math.ceil(len / 2))
           .toString('hex')
           .substring(0, len);
 }
+
+// Compare the password for an user
+// Works with bcrypt and sha1 indipendently
+// An async function is used so that a promise is always returned
+// even for comparing sha1 hashed passwords that use a sync method
+export async function compare (user, passwordToCheck) {
+  if (!user || !passwordToCheck) throw new Error('user and passwordToCheck are required parameters.');
+
+  let passwordHashMethod = user.auth.local.passwordHashMethod;
+  let passwordHash = user.auth.local.hashed_password;
+  let passwordSalt = user.auth.local.salt; // Only used for SHA1
+
+  if (passwordHashMethod === 'bcrypt') {
+    return await bcryptCompare(passwordToCheck, passwordHash);
+  // default to sha1 if the user has a salt but no passwordHashMethod
+  } else if (passwordHashMethod === 'sha1' || !passwordHashMethod && passwordSalt) {
+    return passwordHash === sha1Encrypt(passwordToCheck, passwordSalt);
+  } else {
+    throw new Error('Invalid password hash method.');
+  }
+}
+
+// Convert an user to use bcrypt from sha1 for password hashing
+// needs to save the user separately.
+// NOTE: before calling this method it should be verified that the supplied plain text password
+// is indeed hashed with sha1 and is valid
+export async function convertToBcrypt (user, plainTextPassword) {
+  if (!user || !plainTextPassword) throw new Error('user and plainTextPassword are required parameters.');
+
+  user.auth.local.salt = undefined;
+  user.auth.local.passwordHashMethod = 'bcrypt';
+  user.auth.local.hashed_password = await bcryptHash(plainTextPassword); // eslint-disable-line camelcase
+}

website/server/middlewares/index.js

@@ -28,7 +28,7 @@ import {
 } from './language';
 
 const IS_PROD = nconf.get('IS_PROD');
-const DISABLE_LOGGING = nconf.get('DISABLE_REQUEST_LOGGING');
+const DISABLE_LOGGING = nconf.get('DISABLE_REQUEST_LOGGING') === 'true';
 const PUBLIC_DIR = path.join(__dirname, '/../../client-old');
 
 const SESSION_SECRET = nconf.get('SESSION_SECRET');