From dc055e55593860f7cf1d68b55226426d560c96a6 Mon Sep 17 00:00:00 2001 From: Ron Edgecomb Date: Thu, 18 Dec 2014 13:07:31 -0500 Subject: [PATCH 1/2] Require valid login credentials before verified email check. - strongloop/loopback#931. --- common/models/user.js | 43 +++++++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/common/models/user.js b/common/models/user.js index 42661905..85bf389a 100644 --- a/common/models/user.js +++ b/common/models/user.js @@ -183,33 +183,32 @@ module.exports = function(User) { debug('An error is reported from User.findOne: %j', err); fn(defaultError); } else if (user) { - if (self.settings.emailVerificationRequired) { - if (!user.emailVerified) { - // Fail to log in if email verification is not done yet - debug('User email has not been verified'); - err = new Error('login failed as the email has not been verified'); - err.statusCode = 401; - return fn(err); - } - } user.hasPassword(credentials.password, function(err, isMatch) { if (err) { debug('An error is reported from User.hasPassword: %j', err); fn(defaultError); } else if (isMatch) { - user.createAccessToken(credentials.ttl, function(err, token) { - if (err) return fn(err); - if (Array.isArray(include) ? include.indexOf('user') !== -1 : include === 'user') { - // NOTE(bajtos) We can't set token.user here: - // 1. token.user already exists, it's a function injected by - // "AccessToken belongsTo User" relation - // 2. ModelBaseClass.toJSON() ignores own properties, thus - // the value won't be included in the HTTP response - // See also loopback#161 and loopback#162 - token.__data.user = user; - } - fn(err, token); - }); + if (self.settings.emailVerificationRequired && !user.emailVerified) { + // Fail to log in if email verification is not done yet + debug('User email has not been verified'); + err = new Error('login failed as the email has not been verified'); + err.statusCode = 401; + return fn(err); + } else { + user.createAccessToken(credentials.ttl, function(err, token) { + if (err) return fn(err); + if (Array.isArray(include) ? include.indexOf('user') !== -1 : include === 'user') { + // NOTE(bajtos) We can't set token.user here: + // 1. token.user already exists, it's a function injected by + // "AccessToken belongsTo User" relation + // 2. ModelBaseClass.toJSON() ignores own properties, thus + // the value won't be included in the HTTP response + // See also loopback#161 and loopback#162 + token.__data.user = user; + } + fn(err, token); + }); + } } else { debug('The password is invalid for user %s', query.email || query.username); fn(defaultError); From 70f576b452336db9e4e1742847ad038bc7359b05 Mon Sep 17 00:00:00 2001 From: Ron Edgecomb Date: Sun, 21 Dec 2014 22:18:36 -0500 Subject: [PATCH 2/2] API and REST tests added to ensure complete and valid credentials are supplied for verified error message to be returned - tests added as suggested and fail under previous version of User model - strongloop/loopback#931 --- test/user.test.js | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/test/user.test.js b/test/user.test.js index 60ef9182..fffd2a2a 100644 --- a/test/user.test.js +++ b/test/user.test.js @@ -7,7 +7,8 @@ var userMemory = loopback.createDataSource({ }); describe('User', function() { - var validCredentials = {email: 'foo@bar.com', password: 'bar'}; + var validCredentialsEmail = 'foo@bar.com'; + var validCredentials = {email: validCredentialsEmail, password: 'bar'}; var validCredentialsEmailVerified = {email: 'foo1@bar.com', password: 'bar1', emailVerified: true}; var validCredentialsEmailVerifiedOverREST = {email: 'foo2@bar.com', password: 'bar2', emailVerified: true}; var validCredentialsWithTTL = {email: 'foo@bar.com', password: 'bar', ttl: 3600}; @@ -308,6 +309,15 @@ describe('User', function() { User.settings.emailVerificationRequired = false; }); + it('Require valid and complete credentials for email verification error', function(done) { + User.login({ email: validCredentialsEmail }, function(err, accessToken) { + // strongloop/loopback#931 + // error message should be "login failed" and not "login failed as the email has not been verified" + assert(err && !/verified/.test(err.message), ('expecting "login failed" error message, received: "' + err.message + '"')); + done(); + }); + }); + it('Login a user by without email verification', function(done) { User.login(validCredentials, function(err, accessToken) { assert(err); @@ -339,6 +349,21 @@ describe('User', function() { }); }); + it('Login a user over REST require complete and valid credentials for email verification error message', function(done) { + request(app) + .post('/users/login') + .expect('Content-Type', /json/) + .expect(401) + .send({ email: validCredentialsEmail }) + .end(function(err, res) { + // strongloop/loopback#931 + // error message should be "login failed" and not "login failed as the email has not been verified" + var errorResponse = res.body.error; + assert(errorResponse && !/verified/.test(errorResponse.message), ('expecting "login failed" error message, received: "' + errorResponse.message + '"')); + done(); + }); + }); + it('Login a user over REST without email verification when it is required', function(done) { request(app) .post('/users/login')