From 3e45f5af41af3b454df93d754a75fde429348638 Mon Sep 17 00:00:00 2001 From: Chester Sng Date: Fri, 22 Mar 2019 19:30:00 +0800 Subject: [PATCH 1/3] Add check for the existence of user's password before attempting to authenticate --- website/server/controllers/api-v3/auth.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/website/server/controllers/api-v3/auth.js b/website/server/controllers/api-v3/auth.js index 37d7f7e451..84c5485fe6 100644 --- a/website/server/controllers/api-v3/auth.js +++ b/website/server/controllers/api-v3/auth.js @@ -98,6 +98,9 @@ api.loginLocal = { // load the entire user because we may have to save it to convert the password to bcrypt let user = await User.findOne(login).exec(); + // if user is using social login, then user will not have a hashed_password stored + if (!user.auth.local.hashed_password) throw new NotAuthorized(res.t('invalidLoginCredentialsLong')); + let isValidPassword; if (!user) { From d20cd1bbf12de3fec5c4f17b5624dacfe55076b9 Mon Sep 17 00:00:00 2001 From: Chester Sng Date: Fri, 22 Mar 2019 19:36:13 +0800 Subject: [PATCH 2/3] Remove unused string constant in front.js --- website/common/locales/en/front.json | 1 - 1 file changed, 1 deletion(-) diff --git a/website/common/locales/en/front.json b/website/common/locales/en/front.json index c249bc9f51..06ead889ae 100644 --- a/website/common/locales/en/front.json +++ b/website/common/locales/en/front.json @@ -276,7 +276,6 @@ "usernameTOSRequirements": "Usernames must conform to our Terms of Service and Community Guidelines. If you didn’t previously set a login name, your username was auto-generated.", "usernameTaken": "Username already taken.", "passwordConfirmationMatch": "Password confirmation doesn't match password.", - "invalidLoginCredentials": "Incorrect username and/or email and/or password.", "passwordResetPage": "Reset Password", "passwordReset": "If we have your email on file, instructions for setting a new password have been sent to your email.", "passwordResetEmailSubject": "Password Reset for Habitica", From 87d86ee632b79834539d993d91a43c4e55a8eeb0 Mon Sep 17 00:00:00 2001 From: Chester Sng Date: Sat, 23 Mar 2019 00:56:36 +0800 Subject: [PATCH 3/3] Add test case for user that uses social authentication --- .../user/auth/POST-login-local.test.js | 18 ++++++++++++++++++ test/helpers/api-integration/api-classes.js | 13 +++++++++++++ test/helpers/mongo.js | 13 +++++++++++++ 3 files changed, 44 insertions(+) diff --git a/test/api/v3/integration/user/auth/POST-login-local.test.js b/test/api/v3/integration/user/auth/POST-login-local.test.js index ad1eecb778..484ff16af5 100644 --- a/test/api/v3/integration/user/auth/POST-login-local.test.js +++ b/test/api/v3/integration/user/auth/POST-login-local.test.js @@ -110,4 +110,22 @@ describe('POST /user/auth/local/login', () => { let isValidPassword = await bcryptCompare(textPassword, user.auth.local.hashed_password); expect(isValidPassword).to.equal(true); }); + + it('user uses social authentication and has no password', async () => { + await user.unset({ + 'auth.local.hashed_password': 1, + }); + + await user.sync(); + expect(user.auth.local.hashed_password).to.be.undefined; + + await expect(api.post(endpoint, { + username: user.auth.local.username, + password: 'any-password', + })).to.eventually.be.rejected.and.eql({ + code: 401, + error: 'NotAuthorized', + message: t('invalidLoginCredentialsLong'), + }); + }); }); diff --git a/test/helpers/api-integration/api-classes.js b/test/helpers/api-integration/api-classes.js index 7612c88cfd..f914d0d3a5 100644 --- a/test/helpers/api-integration/api-classes.js +++ b/test/helpers/api-integration/api-classes.js @@ -4,6 +4,7 @@ import { requester } from './requester'; import { getDocument as getDocumentFromMongo, updateDocument as updateDocumentInMongo, + unsetDocument as unsetDocumentInMongo, } from '../mongo'; import { assign, @@ -29,6 +30,18 @@ class ApiObject { return this; } + async unset (options) { + if (isEmpty(options)) { + return; + } + + await unsetDocumentInMongo(this._docType, this, options); + + _updateLocalParameters((this, options)); + + return this; + } + async sync () { let updatedDoc = await getDocumentFromMongo(this._docType, this); diff --git a/test/helpers/mongo.js b/test/helpers/mongo.js index fb16e3a24f..ca2af9ce45 100644 --- a/test/helpers/mongo.js +++ b/test/helpers/mongo.js @@ -98,6 +98,19 @@ export async function updateDocument (collectionName, doc, update) { }); } +// Unset a property in the database. +// Useful for testing. +export async function unsetDocument (collectionName, doc, update) { + let collection = mongoose.connection.db.collection(collectionName); + + return new Promise((resolve) => { + collection.updateOne({ _id: doc._id }, { $unset: update }, (updateErr) => { + if (updateErr) throw new Error(`Error updating ${collectionName}: ${updateErr}`); + resolve(); + }); + }); +} + export async function getDocument (collectionName, doc) { let collection = mongoose.connection.db.collection(collectionName);