diff --git a/common/models/user.js b/common/models/user.js index 8a5fdc97..cd10714e 100644 --- a/common/models/user.js +++ b/common/models/user.js @@ -421,15 +421,82 @@ module.exports = function(User) { return cb(err); } - try { - User.validatePassword(newPassword); - } catch (err) { + this.setPassword(newPassword, options, cb); + }); + return cb.promise; + }; + + /** + * Set this user's password after a password-reset request was made. + * + * @param {*} userId Id of the user changing the password + * @param {string} newPassword The new password to use. + * @param {Object} [options] Additional options including remoting context + * @callback {Function} callback + * @param {Error} err Error object + * @promise + */ + User.setPassword = function(userId, newPassword, options, cb) { + assert(userId != null && userId !== '', 'userId is a required argument'); + assert(!!newPassword, 'newPassword is a required argument'); + + if (cb === undefined && typeof options === 'function') { + cb = options; + options = undefined; + } + cb = cb || utils.createPromiseCallback(); + + // Make sure to use the constructor of the (sub)class + // where the method is invoked from (`this` instead of `User`) + this.findById(userId, options, (err, inst) => { + if (err) return cb(err); + + if (!inst) { + const err = new Error(`User ${userId} not found`); + Object.assign(err, { + code: 'USER_NOT_FOUND', + statusCode: 401, + }); return cb(err); } - const delta = {password: newPassword}; - this.patchAttributes(delta, options, (err, updated) => cb(err)); + inst.setPassword(newPassword, options, cb); }); + + return cb.promise; + }; + + /** + * Set this user's password. The callers of this method + * must ensure the client making the request is authorized + * to change the password, typically by providing the correct + * current password or a password-reset token. + * + * @param {string} newPassword The new password to use. + * @param {Object} [options] Additional options including remoting context + * @callback {Function} callback + * @param {Error} err Error object + * @promise + */ + User.prototype.setPassword = function(newPassword, options, cb) { + assert(!!newPassword, 'newPassword is a required argument'); + + if (cb === undefined && typeof options === 'function') { + cb = options; + options = undefined; + } + cb = cb || utils.createPromiseCallback(); + + try { + this.constructor.validatePassword(newPassword); + } catch (err) { + cb(err); + return cb.promise; + } + + const delta = {password: newPassword}; + this.patchAttributes(delta, options, (err, updated) => cb(err)); + return cb.promise; }; @@ -936,6 +1003,21 @@ module.exports = function(User) { } ); + UserModel.remoteMethod( + 'setPassword', + { + description: 'Reset user\'s password via a password-reset token.', + accepts: [ + {arg: 'id', type: 'any', + http: ctx => ctx.req.accessToken && ctx.req.accessToken.userId, + }, + {arg: 'newPassword', type: 'string', required: true, http: {source: 'form'}}, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], + http: {verb: 'POST', path: '/reset-password'}, + } + ); + UserModel.afterRemote('confirm', function(ctx, inst, next) { if (ctx.args.redirect !== undefined) { if (!ctx.res) { diff --git a/common/models/user.json b/common/models/user.json index 230fe7e8..c0e4d66b 100644 --- a/common/models/user.json +++ b/common/models/user.json @@ -89,6 +89,13 @@ "permission": "ALLOW", "property": "changePassword", "accessType": "EXECUTE" + }, + { + "principalType": "ROLE", + "principalId": "$authenticated", + "permission": "ALLOW", + "property": "setPassword", + "accessType": "EXECUTE" } ], "relations": { diff --git a/test/user.integration.js b/test/user.integration.js index 39372c19..bc2f31d4 100644 --- a/test/user.integration.js +++ b/test/user.integration.js @@ -10,6 +10,8 @@ var path = require('path'); var SIMPLE_APP = path.join(__dirname, 'fixtures', 'user-integration-app'); var app = require(path.join(SIMPLE_APP, 'server/server.js')); var expect = require('./helpers/expect'); +const Promise = require('bluebird'); +const waitForEvent = require('./helpers/wait-for-event'); describe('users - integration', function() { lt.beforeEach.withApp(app); @@ -178,6 +180,64 @@ describe('users - integration', function() { expect(injectedOptions).to.have.property('accessToken'); }); }); + + it('resets user\'s password', function() { + const User = app.models.User; + const credentials = {email: 'reset@example.com', password: 'pass'}; + return User.create(credentials) + .then(u => { + this.user = u; + return triggerPasswordReset(credentials.email); + }) + .then(info => { + return this.post('/api/users/reset-password') + .set('Authorization', info.accessToken.id) + .send({ + newPassword: 'new password', + }) + .expect(204); + }) + .then(() => { + return User.findById(this.user.id); + }) + .then(user => { + return user.hasPassword('new password'); + }) + .then(isMatch => expect(isMatch, 'user has new password').to.be.true()); + }); + + it('rejects unauthenticated reset password request', function() { + return this.post('/api/users/reset-password') + .send({ + newPassword: 'new password', + }) + .expect(401); + }); + + it('injects reset password options from remoting context', function() { + const User = app.models.User; + const credentials = {email: 'inject-reset@example.com', password: 'pass'}; + + let injectedOptions; + User.observe('before save', (ctx, next) => { + injectedOptions = ctx.options; + next(); + }); + + return User.create(credentials) + .then(u => triggerPasswordReset(credentials.email)) + .then(info => { + return this.post('/api/users/reset-password') + .set('Authorization', info.accessToken.id) + .send({ + newPassword: 'new password', + }) + .expect(204); + }) + .then(() => { + expect(injectedOptions).to.have.property('accessToken'); + }); + }); }); describe('sub-user', function() { @@ -246,4 +306,13 @@ describe('users - integration', function() { }); }); }); + + function triggerPasswordReset(email) { + const User = app.models.User; + return Promise.all([ + User.resetPassword({email: email}), + waitForEvent(app.models.User, 'resetPasswordRequest'), + ]) + .spread((reset, info) => info); + } }); diff --git a/test/user.test.js b/test/user.test.js index 65008dc2..643058bb 100644 --- a/test/user.test.js +++ b/test/user.test.js @@ -471,6 +471,25 @@ describe('User', function() { }); }); }); + + it('rejects setPassword when new password is longer than 72 chars', function() { + return User.create({email: 'test@example.com', password: pass72Char}) + .then(u => u.setPassword(pass73Char)) + .then( + success => { throw new Error('setPassword should have failed'); }, + err => { + expect(err.message).to.match(/Password too long/); + + // workaround for chai problem + // object tested must be an array, an object, or a string, + // but error given + const props = Object.assign({}, err); + expect(props).to.contain({ + code: 'PASSWORD_TOO_LONG', + statusCode: 422, + }); + }); + }); }); describe('Access-hook for queries with email NOT case-sensitive', function() { @@ -1406,6 +1425,92 @@ describe('User', function() { } }); + describe('User.setPassword()', () => { + let userId; + beforeEach(givenUserId); + + it('changes the password - callback-style', done => { + User.setPassword(userId, 'new password', (err) => { + if (err) return done(err); + expect(arguments.length, 'changePassword callback arguments length') + .to.be.at.most(1); + + User.findById(userId, (err, user) => { + if (err) return done(err); + user.hasPassword('new password', (err, isMatch) => { + if (err) return done(err); + expect(isMatch, 'user has new password').to.be.true(); + done(); + }); + }); + }); + }); + + it('changes the password - Promise-style', () => { + return User.setPassword(userId, 'new password') + .then(() => { + expect(arguments.length, 'changePassword promise resolution') + .to.equal(0); + return User.findById(userId); + }) + .then(user => { + return user.hasPassword('new password'); + }) + .then(isMatch => { + expect(isMatch, 'user has new password').to.be.true(); + }); + }); + + it('fails with 401 for unknown user', () => { + return User.setPassword('unknown-id', 'pass').then( + success => { throw new Error('setPassword should have failed'); }, + err => { + // workaround for chai problem + // object tested must be an array, an object, or a string, + // but error given + const props = Object.assign({}, err); + expect(props).to.contain({ + code: 'USER_NOT_FOUND', + statusCode: 401, + }); + }); + }); + + it('forwards the "options" argument', () => { + const options = {testFlag: true}; + const observedOptions = []; + + saveObservedOptionsForHook('access'); + saveObservedOptionsForHook('before save'); + + return User.setPassword(userId, 'new', options) + .then(() => expect(observedOptions).to.eql([ + // findById + {hook: 'access', testFlag: true}, + + // "before save" hook prepareForTokenInvalidation + {hook: 'access', testFlag: true}, + + // updateAttributes + {hook: 'before save', testFlag: true}, + + // validate uniqueness of User.email + {hook: 'access', testFlag: true}, + ])); + + function saveObservedOptionsForHook(name) { + User.observe(name, (ctx, next) => { + observedOptions.push(Object.assign({hook: name}, ctx.options)); + next(); + }); + } + }); + + function givenUserId() { + userId = validCredentialsUser.id; + } + }); + describe('Verification', function() { describe('user.verify(verifyOptions, options, cb)', function() { const ctxOptions = {testFlag: true};