Merge pull request #3350 from strongloop/feature/set-password-with-token

Set password with token, disable password changes via patch/replace
This commit is contained in:
Miroslav Bajtoš 2017-04-20 10:47:04 +02:00 committed by GitHub
commit b96605c63a
7 changed files with 684 additions and 27 deletions

View File

@ -79,12 +79,26 @@ module.exports = function(User) {
* Create access token for the logged in user. This method can be overridden to * Create access token for the logged in user. This method can be overridden to
* customize how access tokens are generated * customize how access tokens are generated
* *
* @param {Number} ttl The requested ttl * Supported flavours:
* @param {Object} [options] The options for access token, such as scope, appId *
* ```js
* createAccessToken(ttl, cb)
* createAccessToken(ttl, options, cb);
* createAccessToken(options, cb);
* // recent addition:
* createAccessToken(data, options, cb);
* ```
*
* @options {Number|Object} [ttl|data] Either the requested ttl,
* or an object with token properties to set (see below).
* @property {Number} [ttl] The requested ttl
* @property {String[]} [scopes] The access scopes granted to the token.
* @param {Object} [options] Additional options including remoting context
* @callback {Function} cb The callback function * @callback {Function} cb The callback function
* @param {String|Error} err The error string or object * @param {String|Error} err The error string or object
* @param {AccessToken} token The generated access token object * @param {AccessToken} token The generated access token object
* @promise * @promise
*
*/ */
User.prototype.createAccessToken = function(ttl, options, cb) { User.prototype.createAccessToken = function(ttl, options, cb) {
if (cb === undefined && typeof options === 'function') { if (cb === undefined && typeof options === 'function') {
@ -95,17 +109,21 @@ module.exports = function(User) {
cb = cb || utils.createPromiseCallback(); cb = cb || utils.createPromiseCallback();
if (typeof ttl === 'object' && !options) { let tokenData;
// createAccessToken(options, cb) if (typeof ttl !== 'object') {
options = ttl; // createAccessToken(ttl[, options], cb)
ttl = options.ttl; tokenData = {ttl};
} else if (options) {
// createAccessToken(data, options, cb)
tokenData = ttl;
} else {
// createAccessToken(options, cb);
tokenData = {};
} }
options = options || {};
var userModel = this.constructor; var userSettings = this.constructor.settings;
ttl = Math.min(ttl || userModel.settings.ttl, userModel.settings.maxTTL); tokenData.ttl = Math.min(tokenData.ttl || userSettings.ttl, userSettings.maxTTL);
this.accessTokens.create({ this.accessTokens.create(tokenData, options, cb);
ttl: ttl,
}, cb);
return cb.promise; return cb.promise;
}; };
@ -421,15 +439,90 @@ module.exports = function(User) {
return cb(err); return cb(err);
} }
try { this.setPassword(newPassword, options, cb);
User.validatePassword(newPassword); });
} catch (err) { 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); return cb(err);
} }
const delta = {password: newPassword}; inst.setPassword(newPassword, options, cb);
this.patchAttributes(delta, options, (err, updated) => cb(err));
}); });
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;
}
// We need to modify options passed to patchAttributes, but we don't want
// to modify the original options object passed to us by setPassword caller
options = Object.assign({}, options);
// patchAttributes() does not allow callers to modify the password property
// unless "options.setPassword" is set.
options.setPassword = true;
const delta = {password: newPassword};
this.patchAttributes(delta, options, (err, updated) => cb(err));
return cb.promise; return cb.promise;
}; };
@ -739,7 +832,21 @@ module.exports = function(User) {
return cb(err); return cb(err);
} }
user.createAccessToken(ttl, function(err, accessToken) { if (UserModel.settings.restrictResetPasswordTokenScope) {
const tokenData = {
ttl: ttl,
scopes: ['reset-password'],
};
user.createAccessToken(tokenData, options, onTokenCreated);
} else {
// We need to preserve backwards-compatibility with
// user-supplied implementations of "createAccessToken"
// that may not support "options" argument (we have such
// examples in our test suite).
user.createAccessToken(ttl, onTokenCreated);
}
function onTokenCreated(err, accessToken) {
if (err) { if (err) {
return cb(err); return cb(err);
} }
@ -750,7 +857,7 @@ module.exports = function(User) {
user: user, user: user,
options: options, options: options,
}); });
}); }
}); });
return cb.promise; return cb.promise;
@ -936,6 +1043,25 @@ module.exports = function(User) {
} }
); );
const setPasswordScopes = UserModel.settings.restrictResetPasswordTokenScope ?
['reset-password'] : undefined;
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'},
],
accessScopes: setPasswordScopes,
http: {verb: 'POST', path: '/reset-password'},
}
);
UserModel.afterRemote('confirm', function(ctx, inst, next) { UserModel.afterRemote('confirm', function(ctx, inst, next) {
if (ctx.args.redirect !== undefined) { if (ctx.args.redirect !== undefined) {
if (!ctx.res) { if (!ctx.res) {
@ -997,6 +1123,46 @@ module.exports = function(User) {
next(); next();
}); });
User.observe('before save', function rejectInsecurePasswordChange(ctx, next) {
const UserModel = ctx.Model;
if (!UserModel.settings.rejectPasswordChangesViaPatchOrReplace) {
// In legacy password flow, any DAO method can change the password
return next();
}
if (ctx.isNewInstance) {
// The password can be always set when creating a new User instance
return next();
}
const data = ctx.data || ctx.instance;
const isPasswordChange = 'password' in data;
// This is the option set by `setPassword()` API
// when calling `this.patchAttritubes()` to change user's password
if (ctx.options.setPassword) {
// Verify that only the password is changed and nothing more or less.
if (Object.keys(data).length > 1 || !isPasswordChange) {
// This is a programmer's error, use the default status code 500
return next(new Error(
'Invalid use of "options.setPassword". Only "password" can be ' +
'changed when using this option.'));
}
return next();
}
if (!isPasswordChange) {
return next();
}
const err = new Error(
'Changing user password via patch/replace API is not allowed. ' +
'Use changePassword() or setPassword() instead.');
err.statusCode = 401;
err.code = 'PASSWORD_CHANGE_NOT_ALLOWED';
next(err);
});
User.observe('before save', function prepareForTokenInvalidation(ctx, next) { User.observe('before save', function prepareForTokenInvalidation(ctx, next) {
if (ctx.isNewInstance) return next(); if (ctx.isNewInstance) return next();
if (!ctx.where && !ctx.instance) return next(); if (!ctx.where && !ctx.instance) return next();

View File

@ -89,6 +89,13 @@
"permission": "ALLOW", "permission": "ALLOW",
"property": "changePassword", "property": "changePassword",
"accessType": "EXECUTE" "accessType": "EXECUTE"
},
{
"principalType": "ROLE",
"principalId": "$authenticated",
"permission": "ALLOW",
"property": "setPassword",
"accessType": "EXECUTE"
} }
], ],
"relations": { "relations": {

View File

@ -82,6 +82,10 @@ describe('Authorization scopes', () => {
} }
function givenRemoteMethodWithCustomScope() { function givenRemoteMethodWithCustomScope() {
// Delete any previously registered instance of the method "scoped"
User.sharedClass._methods = User.sharedClass._methods
.filter(m => m.name !== 'scoped');
const accessScopes = arguments[0] || [CUSTOM_SCOPE]; const accessScopes = arguments[0] || [CUSTOM_SCOPE];
User.scoped = function(cb) { cb(); }; User.scoped = function(cb) { cb(); };
User.remoteMethod('scoped', { User.remoteMethod('scoped', {

View File

@ -0,0 +1,17 @@
// Copyright IBM Corp. 2013,2016. All Rights Reserved.
// Node module: loopback
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT
'use strict';
const Promise = require('bluebird');
function waitForEvent(emitter, event) {
return new Promise((resolve, reject) => {
emitter.on(event, resolve);
});
}
module.exports = waitForEvent;

242
test/user-password.test.js Normal file
View File

@ -0,0 +1,242 @@
// Copyright IBM Corp. 2013,2017. All Rights Reserved.
// Node module: loopback
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT
'use strict';
const expect = require('./helpers/expect');
const errorHandler = require('strong-error-handler');
const loopback = require('../');
const Promise = require('bluebird');
const request = require('supertest');
const waitForEvent = require('./helpers/wait-for-event');
describe('User.password', () => {
const credentials = {email: 'test@example.com', password: 'pass'};
let app, User, testUser, regularToken, resetToken;
context('restrict reset password token scope', () => {
beforeEach(givenAppWithRestrictionEnabled);
context('using regular access token', () => {
beforeEach(givenRegularAccessToken);
it('allows patching user name', () => {
return changeName(regularToken).expect(200);
});
it('allows patching user password', () => {
return patchPassword(regularToken).expect(200);
});
it('allows changing user password', () => {
return changePassword(regularToken).expect(204);
});
it('denies resetting user password', () => {
return resetPassword(regularToken).expect(401);
});
});
context('using password-reset token', () => {
beforeEach(givenResetPasswordToken);
it('denies patching user name', () => {
return changeName(resetToken).expect(401);
});
it('denies patching user password', () => {
return patchPassword(resetToken).expect(401);
});
it('denies changing user password', () => {
return changePassword(resetToken).expect(401);
});
it('allows resetting user password', () => {
return resetPassword(resetToken).expect(204);
});
});
function givenAppWithRestrictionEnabled() {
return givenAppWithUser({restrictResetPasswordTokenScope: true});
}
});
context('reject password changes via patch or replace', () => {
beforeEach(givenAppWithRejectionEnabled);
beforeEach(givenRegularAccessToken);
it('allows patching user name', () => {
return changeName(regularToken).expect(200);
});
it('denies patching user password', () => {
return patchPassword(regularToken).expect(401);
});
it('allows changing user password', () => {
return changePassword(regularToken).expect(204);
});
it('denies setPassword-like call with non-password changes', () => {
return patchNameAndPasswordDirectly().then(
function onSuccess() {
throw new Error('patchAttributes() should have failed');
},
function onError(err) {
expect(err.message).to.match(/Invalid use.*options.setPassword/);
});
});
function givenAppWithRejectionEnabled() {
return givenAppWithUser({rejectPasswordChangesViaPatchOrReplace: true});
}
});
context('all feature flags disabled', () => {
beforeEach(givenAppWithNoRestrictions);
context('using regular access token', () => {
beforeEach(givenRegularAccessToken);
it('allows changing user name', () => {
return changeName(regularToken).expect(200);
});
it('allows patching user password', () => {
return patchPassword(regularToken).expect(200);
});
it('allows changing user password', () => {
return changePassword(regularToken).expect(204);
});
it('allows resetting user password', () => {
return resetPassword(regularToken).expect(204);
});
});
context('using password-reset token', () => {
beforeEach(givenResetPasswordToken);
it('allows changing user name', () => {
return changeName(resetToken).expect(200);
});
it('allows patching user password', () => {
return patchPassword(resetToken).expect(200);
});
it('allows changing user password', () => {
return changePassword(resetToken).expect(204);
});
it('allows resetting user password', () => {
return resetPassword(resetToken).expect(204);
});
});
it('allows setPassword-like call with non-password changes', () => {
return patchNameAndPasswordDirectly().then(() => {
// test passed
});
});
function givenAppWithNoRestrictions() {
return givenAppWithUser({
rejectPasswordChangesViaPatchOrReplace: false,
restrictResetPasswordTokenScope: false,
});
}
});
function givenAppWithUser(userSettings) {
app = loopback({localRegistry: true, loadBuiltinModels: true});
app.set('remoting', {rest: {handleErrors: false}});
app.dataSource('db', {connector: 'memory'});
userSettings = Object.assign({
name: 'PwdUser',
base: 'User',
properties: {
name: 'string',
},
// Speed up the password hashing algorithm for tests
saltWorkFactor: 4,
http: {path: '/users'},
}, userSettings);
User = app.registry.createModel(userSettings);
app.model(User, {dataSource: 'db'});
const AccessToken = app.registry.getModel('AccessToken');
AccessToken.settings.relations.user.model = User.modelName;
app.enableAuth({dataSource: 'db'});
app.use(loopback.token());
app.use(loopback.rest());
app.use(function logUnexpectedError(err, req, res, next) {
const statusCode = err.statusCode || err.status;
if (statusCode > 400 && statusCode !== 401) {
console.log('Unexpected error for %s %s: %s %s',
req.method, req.path, statusCode, err.stack || err);
}
next(err);
});
app.use(errorHandler({debug: true, log: false}));
return User.create(credentials)
.then(u => {
testUser = u;
return u.setAttribute('emailVerified', true);
});
}
function givenRegularAccessToken() {
return User.login(credentials).then(t => regularToken = t);
}
function givenResetPasswordToken() {
return Promise.all([
User.resetPassword({email: credentials.email}),
waitForEvent(User, 'resetPasswordRequest'),
])
.spread((reset, info) => resetToken = info.accessToken);
}
function changeName(token) {
return request(app).patch(`/users/${testUser.id}`)
.set('Authorization', token.id)
.send({name: 'New Name'});
}
function patchPassword(token) {
return request(app).patch(`/users/${testUser.id}`)
.set('Authorization', token.id)
.send({password: 'new-pass'});
}
function changePassword(token) {
return request(app).post('/users/change-password')
.set('Authorization', token.id)
.send({oldPassword: credentials.password, newPassword: 'new-pass'});
}
function resetPassword(token) {
return request(app).post('/users/reset-password')
.set('Authorization', token.id)
.send({newPassword: 'new-pass'});
}
function patchNameAndPasswordDirectly() {
return testUser.patchAttributes(
{password: 'new-pass', name: 'New Name'},
{setPassword: true});
}
});

View File

@ -10,6 +10,8 @@ var path = require('path');
var SIMPLE_APP = path.join(__dirname, 'fixtures', 'user-integration-app'); var SIMPLE_APP = path.join(__dirname, 'fixtures', 'user-integration-app');
var app = require(path.join(SIMPLE_APP, 'server/server.js')); var app = require(path.join(SIMPLE_APP, 'server/server.js'));
var expect = require('./helpers/expect'); var expect = require('./helpers/expect');
const Promise = require('bluebird');
const waitForEvent = require('./helpers/wait-for-event');
describe('users - integration', function() { describe('users - integration', function() {
lt.beforeEach.withApp(app); lt.beforeEach.withApp(app);
@ -153,13 +155,13 @@ describe('users - integration', function() {
.expect(401); .expect(401);
}); });
it('injects change password options from remoting context', function() { it('uses change password options provided by the remoting context', function() {
const User = app.models.User; const User = app.models.User;
const credentials = {email: 'inject@example.com', password: 'pass'}; const credentials = {email: 'inject@example.com', password: 'pass'};
let injectedOptions; let observedOptions;
User.observe('before save', (ctx, next) => { User.observe('before save', (ctx, next) => {
injectedOptions = ctx.options; observedOptions = ctx.options;
next(); next();
}); });
@ -175,7 +177,65 @@ describe('users - integration', function() {
.expect(204); .expect(204);
}) })
.then(() => { .then(() => {
expect(injectedOptions).to.have.property('accessToken'); expect(observedOptions).to.have.property('accessToken');
});
});
it('resets the 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 requests', function() {
return this.post('/api/users/reset-password')
.send({
newPassword: 'new password',
})
.expect(401);
});
it('uses password reset options provided by the remoting context', function() {
const User = app.models.User;
const credentials = {email: 'inject-reset@example.com', password: 'pass'};
let observedOptions;
User.observe('before save', (ctx, next) => {
observedOptions = 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(observedOptions).to.have.property('accessToken');
}); });
}); });
}); });
@ -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);
}
}); });

View File

@ -11,7 +11,8 @@ var loopback = require('../');
var async = require('async'); var async = require('async');
var url = require('url'); var url = require('url');
var extend = require('util')._extend; var extend = require('util')._extend;
var Promise = require('bluebird'); const Promise = require('bluebird');
const waitForEvent = require('./helpers/wait-for-event');
var User, AccessToken; var User, AccessToken;
@ -470,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() { describe('Access-hook for queries with email NOT case-sensitive', function() {
@ -1382,13 +1402,13 @@ describe('User', function() {
{hook: 'access', testFlag: true}, {hook: 'access', testFlag: true},
// "before save" hook prepareForTokenInvalidation // "before save" hook prepareForTokenInvalidation
{hook: 'access', testFlag: true}, {hook: 'access', setPassword: true, testFlag: true},
// updateAttributes // updateAttributes
{hook: 'before save', testFlag: true}, {hook: 'before save', setPassword: true, testFlag: true},
// validate uniqueness of User.email // validate uniqueness of User.email
{hook: 'access', testFlag: true}, {hook: 'access', setPassword: true, testFlag: true},
])); ]));
function saveObservedOptionsForHook(name) { function saveObservedOptionsForHook(name) {
@ -1405,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 users', () => {
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', setPassword: true, testFlag: true},
// updateAttributes
{hook: 'before save', setPassword: true, testFlag: true},
// validate uniqueness of User.email
{hook: 'access', setPassword: true, 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('Verification', function() {
describe('user.verify(verifyOptions, options, cb)', function() { describe('user.verify(verifyOptions, options, cb)', function() {
const ctxOptions = {testFlag: true}; const ctxOptions = {testFlag: true};
@ -2165,6 +2271,44 @@ describe('User', function() {
}); });
}); });
it('creates token that allows patching User with new password', () => {
return triggerPasswordReset(options.email)
.then(info => {
// Make a REST request to change the password
return request(app)
.patch(`/test-users/${info.user.id}`)
.set('Authorization', info.accessToken.id)
.send({password: 'new-pass'})
.expect(200);
})
.then(() => {
// Call login to verify the password was changed
const credentials = {email: options.email, password: 'new-pass'};
return User.login(credentials);
});
});
it('creates token that allows calling other endpoints too', () => {
// Setup a test method that can be executed by $owner only
User.prototype.testMethod = function(cb) { cb(null, 'ok'); };
User.remoteMethod('prototype.testMethod', {
returns: {arg: 'status', type: 'string'},
http: {verb: 'get', path: '/test'},
});
User.settings.acls.push({
principalType: 'ROLE',
principalId: '$owner',
permission: 'ALLOW',
property: 'testMethod',
});
return triggerPasswordReset(options.email)
.then(info => request(app)
.get(`/test-users/${info.user.id}/test`)
.set('Authorization', info.accessToken.id)
.expect(200));
});
describe('User.resetPassword(options, cb) requiring realm', function() { describe('User.resetPassword(options, cb) requiring realm', function() {
var realmUser; var realmUser;
@ -2788,4 +2932,12 @@ describe('User', function() {
expect(User2.settings.ttl).to.equal(10); expect(User2.settings.ttl).to.equal(10);
}); });
}); });
function triggerPasswordReset(email) {
return Promise.all([
User.resetPassword({email: email}),
waitForEvent(User, 'resetPasswordRequest'),
])
.spread((reset, info) => info);
}
}); });