diff --git a/CHANGES.md b/CHANGES.md index e0f5a4f7..7915899b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,17 @@ +2015-01-08, Version 2.10.0 +========================== + + * Revert the peer dep change to avoid npm complaints (Raymond Feng) + + * Update strong-remoting dep (Raymond Feng) + + * Allow accessType per remote method (Raymond Feng) + + * 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 (Ron Edgecomb) + + * Require valid login credentials before verified email check. - strongloop/loopback#931. (Ron Edgecomb) + + 2015-01-07, Version 2.9.0 ========================= diff --git a/common/models/user.js b/common/models/user.js index bee83c23..d3422d2f 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); diff --git a/lib/model.js b/lib/model.js index 3ea73d9b..dd76b437 100644 --- a/lib/model.js +++ b/lib/model.js @@ -314,6 +314,25 @@ Model._getAccessTypeForMethod = function(method) { var ACL = Model._ACL(); + // Check the explicit setting of accessType + if (method.accessType) { + assert(method.accessType === ACL.READ || + method.accessType === ACL.WRITE || + method.accessType === ACL.EXECUTE, 'invalid accessType ' + + method.accessType + + '. It must be "READ", "WRITE", or "EXECUTE"'); + return method.accessType; + } + + // Default GET requests to READ + var verb = method.http && method.http.verb; + if (typeof verb === 'string') { + verb = verb.toUpperCase(); + } + if (verb === 'GET' || verb === 'HEAD') { + return ACL.READ; + } + switch (method.name) { case'create': return ACL.WRITE; @@ -406,6 +425,7 @@ Model.belongsToRemoting = function(relationName, relation, define) { isStatic: false, http: {verb: 'get', path: '/' + pathName}, accepts: {arg: 'refresh', type: 'boolean', http: {source: 'query'}}, + accessType: 'READ', description: 'Fetches belongsTo relation ' + relationName, returns: {arg: relationName, type: modelName, root: true} }, fn); @@ -419,6 +439,7 @@ Model.hasOneRemoting = function(relationName, relation, define) { http: {verb: 'get', path: '/' + pathName}, accepts: {arg: 'refresh', type: 'boolean', http: {source: 'query'}}, description: 'Fetches hasOne relation ' + relationName, + accessType: 'READ', returns: {arg: relationName, type: relation.modelTo.modelName, root: true} }, fn); }; @@ -445,6 +466,7 @@ Model.hasManyRemoting = function(relationName, relation, define) { description: 'Foreign key for ' + relationName, required: true, http: {source: 'path'}}, description: 'Find a related item by id for ' + relationName, + accessType: 'READ', returns: {arg: 'result', type: toModelName, root: true}, rest: {after: convertNullToNotFoundError} }, findByIdFunc); @@ -457,6 +479,7 @@ Model.hasManyRemoting = function(relationName, relation, define) { description: 'Foreign key for ' + relationName, required: true, http: {source: 'path'}}, description: 'Delete a related item by id for ' + relationName, + accessType: 'WRITE', returns: [] }, destroyByIdFunc); @@ -471,6 +494,7 @@ Model.hasManyRemoting = function(relationName, relation, define) { {arg: 'data', type: toModelName, http: {source: 'body'}} ], description: 'Update a related item by id for ' + relationName, + accessType: 'WRITE', returns: {arg: 'result', type: toModelName, root: true} }, updateByIdFunc); @@ -491,6 +515,7 @@ Model.hasManyRemoting = function(relationName, relation, define) { description: 'Foreign key for ' + relationName, required: true, http: {source: 'path'}}].concat(accepts), description: 'Add a related item by id for ' + relationName, + accessType: 'WRITE', returns: {arg: relationName, type: modelThrough.modelName, root: true} }, addFunc); @@ -502,6 +527,7 @@ Model.hasManyRemoting = function(relationName, relation, define) { description: 'Foreign key for ' + relationName, required: true, http: {source: 'path'}}, description: 'Remove the ' + relationName + ' relation to an item by id', + accessType: 'WRITE', returns: [] }, removeFunc); @@ -515,6 +541,7 @@ Model.hasManyRemoting = function(relationName, relation, define) { description: 'Foreign key for ' + relationName, required: true, http: {source: 'path'}}, description: 'Check the existence of ' + relationName + ' relation to an item by id', + accessType: 'READ', returns: {arg: 'exists', type: 'boolean', root: true}, rest: { // After hook to map exists to 200/404 for HEAD @@ -556,6 +583,7 @@ Model.scopeRemoting = function(scopeName, scope, define) { http: {verb: 'get', path: '/' + pathName}, accepts: {arg: 'filter', type: 'object'}, description: 'Queries ' + scopeName + ' of ' + this.modelName + '.', + accessType: 'READ', returns: {arg: scopeName, type: [toModelName], root: true} }); @@ -564,13 +592,15 @@ Model.scopeRemoting = function(scopeName, scope, define) { http: {verb: 'post', path: '/' + pathName}, accepts: {arg: 'data', type: toModelName, http: {source: 'body'}}, description: 'Creates a new instance in ' + scopeName + ' of this model.', + accessType: 'WRITE', returns: {arg: 'data', type: toModelName, root: true} }); define('__delete__' + scopeName, { isStatic: isStatic, http: {verb: 'delete', path: '/' + pathName}, - description: 'Deletes all ' + scopeName + ' of this model.' + description: 'Deletes all ' + scopeName + ' of this model.', + accessType: 'WRITE' }); define('__count__' + scopeName, { @@ -578,6 +608,7 @@ Model.scopeRemoting = function(scopeName, scope, define) { http: {verb: 'get', path: '/' + pathName + '/count'}, accepts: {arg: 'where', type: 'object', description: 'Criteria to match model instances'}, description: 'Counts ' + scopeName + ' of ' + this.modelName + '.', + accessType: 'READ', returns: {arg: 'count', type: 'number'} }); @@ -653,6 +684,7 @@ Model.nestRemoting = function(relationName, options, cb) { opts.accepts = acceptArgs.concat(method.accepts || []); opts.returns = [].concat(method.returns || []); opts.description = method.description; + opts.accessType = method.accessType; opts.rest = extend({}, method.rest || {}); opts.rest.delegateTo = method; diff --git a/lib/persisted-model.js b/lib/persisted-model.js index 9bba528d..c6c60374 100644 --- a/lib/persisted-model.js +++ b/lib/persisted-model.js @@ -485,6 +485,7 @@ PersistedModel.setupRemoting = function() { setRemoting(PersistedModel, 'create', { description: 'Create a new instance of the model and persist it into the data source', + accessType: 'WRITE', accepts: {arg: 'data', type: 'object', description: 'Model instance data', http: {source: 'body'}}, returns: {arg: 'data', type: typeName, root: true}, http: {verb: 'post', path: '/'} @@ -493,6 +494,7 @@ PersistedModel.setupRemoting = function() { setRemoting(PersistedModel, 'upsert', { aliases: ['updateOrCreate'], description: 'Update an existing model instance or insert a new one into the data source', + accessType: 'WRITE', accepts: {arg: 'data', type: 'object', description: 'Model instance data', http: {source: 'body'}}, returns: {arg: 'data', type: typeName, root: true}, http: {verb: 'put', path: '/'} @@ -500,6 +502,7 @@ PersistedModel.setupRemoting = function() { setRemoting(PersistedModel, 'exists', { description: 'Check whether a model instance exists in the data source', + accessType: 'READ', accepts: {arg: 'id', type: 'any', description: 'Model id', required: true}, returns: {arg: 'exists', type: 'boolean'}, http: [ @@ -529,6 +532,7 @@ PersistedModel.setupRemoting = function() { setRemoting(PersistedModel, 'findById', { description: 'Find a model instance by id from the data source', + accessType: 'READ', accepts: { arg: 'id', type: 'any', description: 'Model id', required: true, http: {source: 'path'} @@ -540,6 +544,7 @@ PersistedModel.setupRemoting = function() { setRemoting(PersistedModel, 'find', { description: 'Find all instances of the model matched by filter from the data source', + accessType: 'READ', accepts: {arg: 'filter', type: 'object', description: 'Filter defining fields, where, orderBy, offset, and limit'}, returns: {arg: 'data', type: [typeName], root: true}, http: {verb: 'get', path: '/'} @@ -547,6 +552,7 @@ PersistedModel.setupRemoting = function() { setRemoting(PersistedModel, 'findOne', { description: 'Find first instance of the model matched by filter from the data source', + accessType: 'READ', accepts: {arg: 'filter', type: 'object', description: 'Filter defining fields, where, orderBy, offset, and limit'}, returns: {arg: 'data', type: typeName, root: true}, http: {verb: 'get', path: '/findOne'}, @@ -555,6 +561,7 @@ PersistedModel.setupRemoting = function() { setRemoting(PersistedModel, 'destroyAll', { description: 'Delete all matching records', + accessType: 'WRITE', accepts: {arg: 'where', type: 'object', description: 'filter.where object'}, http: {verb: 'del', path: '/'}, shared: false @@ -563,6 +570,7 @@ PersistedModel.setupRemoting = function() { setRemoting(PersistedModel, 'updateAll', { aliases: ['update'], description: 'Update instances of the model matched by where from the data source', + accessType: 'WRITE', accepts: [ {arg: 'where', type: 'object', http: {source: 'query'}, description: 'Criteria to match model instances'}, @@ -575,6 +583,7 @@ PersistedModel.setupRemoting = function() { setRemoting(PersistedModel, 'deleteById', { aliases: ['destroyById', 'removeById'], description: 'Delete a model instance by id from the data source', + accessType: 'WRITE', accepts: {arg: 'id', type: 'any', description: 'Model id', required: true, http: {source: 'path'}}, http: {verb: 'del', path: '/:id'} @@ -582,6 +591,7 @@ PersistedModel.setupRemoting = function() { setRemoting(PersistedModel, 'count', { description: 'Count instances of the model matched by where from the data source', + accessType: 'READ', accepts: {arg: 'where', type: 'object', description: 'Criteria to match model instances'}, returns: {arg: 'count', type: 'number'}, http: {verb: 'get', path: '/count'} @@ -589,6 +599,7 @@ PersistedModel.setupRemoting = function() { setRemoting(PersistedModel.prototype, 'updateAttributes', { description: 'Update attributes for a model instance and persist it into the data source', + accessType: 'WRITE', accepts: {arg: 'data', type: 'object', http: {source: 'body'}, description: 'An object of model property name/value pairs'}, returns: {arg: 'data', type: typeName, root: true}, http: {verb: 'put', path: '/'} @@ -597,6 +608,7 @@ PersistedModel.setupRemoting = function() { if (options.trackChanges) { setRemoting(PersistedModel, 'diff', { description: 'Get a set of deltas and conflicts since the given checkpoint', + accessType: 'READ', accepts: [ {arg: 'since', type: 'number', description: 'Find deltas since this checkpoint'}, {arg: 'remoteChanges', type: 'array', description: 'an array of change objects', @@ -609,6 +621,7 @@ PersistedModel.setupRemoting = function() { setRemoting(PersistedModel, 'changes', { description: 'Get the changes to a model since a given checkpoint.' + 'Provide a filter object to reduce the number of results returned.', + accessType: 'READ', accepts: [ {arg: 'since', type: 'number', description: 'Only return changes since this checkpoint'}, {arg: 'filter', type: 'object', description: 'Only include changes that match this filter'} @@ -619,18 +632,21 @@ PersistedModel.setupRemoting = function() { setRemoting(PersistedModel, 'checkpoint', { description: 'Create a checkpoint.', + accessType: 'WRITE', returns: {arg: 'checkpoint', type: 'object', root: true}, http: {verb: 'post', path: '/checkpoint'} }); setRemoting(PersistedModel, 'currentCheckpoint', { description: 'Get the current checkpoint.', + accessType: 'READ', returns: {arg: 'checkpoint', type: 'object', root: true}, http: {verb: 'get', path: '/checkpoint'} }); setRemoting(PersistedModel, 'createUpdates', { description: 'Create an update list from a delta list', + accessType: 'WRITE', accepts: {arg: 'deltas', type: 'array', http: {source: 'body'}}, returns: {arg: 'updates', type: 'array', root: true}, http: {verb: 'post', path: '/create-updates'} @@ -638,17 +654,20 @@ PersistedModel.setupRemoting = function() { setRemoting(PersistedModel, 'bulkUpdate', { description: 'Run multiple updates at once. Note: this is not atomic.', + accessType: 'WRITE', accepts: {arg: 'updates', type: 'array'}, http: {verb: 'post', path: '/bulk-update'} }); setRemoting(PersistedModel, 'rectifyAllChanges', { description: 'Rectify all Model changes.', + accessType: 'WRITE', http: {verb: 'post', path: '/rectify-all'} }); setRemoting(PersistedModel, 'rectifyChange', { description: 'Tell loopback that a change to the model with the given id has occurred.', + accessType: 'WRITE', accepts: {arg: 'id', type: 'any', http: {source: 'path'}}, http: {verb: 'post', path: '/:id/rectify-change'} }); diff --git a/package.json b/package.json index 63a738e9..d087afb6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "loopback", - "version": "2.9.0", + "version": "2.10.0", "description": "LoopBack: Open Source Framework for Node.js", "homepage": "http://loopback.io", "keywords": [ @@ -47,12 +47,12 @@ "nodemailer-stub-transport": "~0.1.4", "serve-favicon": "^2.1.6", "stable": "^0.1.5", - "strong-remoting": "^2.4.0", + "strong-remoting": "^2.11.0", "uid2": "0.0.3", "underscore.string": "~2.3.3" }, "peerDependencies": { - "loopback-datasource-juggler": "^2.13.0" + "loopback-datasource-juggler": "^2.8.0" }, "devDependencies": { "browserify": "~4.2.3", @@ -102,6 +102,6 @@ "url": "https://github.com/strongloop/loopback/blob/master/LICENSE" }, "optionalDependencies": { - "sl-blip": "http://blip.strongloop.com/loopback@2.9.0" + "sl-blip": "http://blip.strongloop.com/loopback@2.10.0" } } diff --git a/test/user.test.js b/test/user.test.js index 700277b0..022af58c 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}; @@ -364,6 +365,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); @@ -397,6 +407,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')