Add User.changePassword(id, old, new, cb)
Implement a new method for changing user passwords the secure way.
The method requires the old password to be provided before a new
password can be used.
REST API:
POST /api/users/change-password
Authorization: your-token-id
Content-Type: application/json
{"oldPassword":"old-pass", "newPassword": "new-pass"}
JavaScript API:
User.changePassword(userId, oldPassword, newPassword[, cb])
There is also an instance-level (prototype) method that can be used
from JavaScript:
userInstance.changePassword(oldPassword, newPassword[, cb])
This commit is contained in:
Miroslav Bajtoš
parent
cb7e2114ec
commit
27ed712528
|
|
@ -353,6 +353,80 @@ module.exports = function(User) {
|
|||
return fn.promise;
|
||||
};
|
||||
|
||||
/**
|
||||
* Change this user's password.
|
||||
*
|
||||
* @param {*} userId Id of the user changing the password
|
||||
* @param {string} oldPassword Current password, required in order
|
||||
* to strongly verify the identity of the requesting user
|
||||
* @param {string} newPassword The new password to use.
|
||||
* @param {object} [options]
|
||||
* @callback {Function} callback
|
||||
* @param {Error} err Error object
|
||||
* @promise
|
||||
*/
|
||||
User.changePassword = function(userId, oldPassword, newPassword, options, cb) {
|
||||
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);
|
||||
}
|
||||
|
||||
inst.changePassword(oldPassword, newPassword, options, cb);
|
||||
});
|
||||
|
||||
return cb.promise;
|
||||
};
|
||||
|
||||
/**
|
||||
* Change this user's password (prototype/instance version).
|
||||
*
|
||||
* @param {string} oldPassword Current password, required in order
|
||||
* to strongly verify the identity of the requesting user
|
||||
* @param {string} newPassword The new password to use.
|
||||
* @param {object} [options]
|
||||
* @callback {Function} callback
|
||||
* @param {Error} err Error object
|
||||
* @promise
|
||||
*/
|
||||
User.prototype.changePassword = function(oldPassword, newPassword, options, cb) {
|
||||
if (cb === undefined && typeof options === 'function') {
|
||||
cb = options;
|
||||
options = undefined;
|
||||
}
|
||||
cb = cb || utils.createPromiseCallback();
|
||||
|
||||
this.hasPassword(oldPassword, (err, isMatch) => {
|
||||
if (err) return cb(err);
|
||||
if (!isMatch) {
|
||||
const err = new Error('Invalid current password');
|
||||
Object.assign(err, {
|
||||
code: 'INVALID_PASSWORD',
|
||||
statusCode: 400,
|
||||
});
|
||||
return cb(err);
|
||||
}
|
||||
|
||||
const delta = {password: newPassword};
|
||||
this.patchAttributes(delta, options, (err, updated) => cb(err));
|
||||
});
|
||||
return cb.promise;
|
||||
};
|
||||
|
||||
/**
|
||||
* Verify a user's identity by sending them a confirmation email.
|
||||
*
|
||||
|
|
@ -812,6 +886,22 @@ module.exports = function(User) {
|
|||
}
|
||||
);
|
||||
|
||||
UserModel.remoteMethod(
|
||||
'changePassword',
|
||||
{
|
||||
description: 'Change a user\'s password.',
|
||||
accepts: [
|
||||
{arg: 'id', type: 'any',
|
||||
http: ctx => ctx.req.accessToken && ctx.req.accessToken.userId,
|
||||
},
|
||||
{arg: 'oldPassword', type: 'string', required: true, http: {source: 'form'}},
|
||||
{arg: 'newPassword', type: 'string', required: true, http: {source: 'form'}},
|
||||
{arg: 'options', type: 'object', http: 'optionsFromRequest'},
|
||||
],
|
||||
http: {verb: 'POST', path: '/change-password'},
|
||||
}
|
||||
);
|
||||
|
||||
UserModel.afterRemote('confirm', function(ctx, inst, next) {
|
||||
if (ctx.args.redirect !== undefined) {
|
||||
if (!ctx.res) {
|
||||
|
|
|
|||
|
|
@ -82,6 +82,13 @@
|
|||
"permission": "ALLOW",
|
||||
"property": "resetPassword",
|
||||
"accessType": "EXECUTE"
|
||||
},
|
||||
{
|
||||
"principalType": "ROLE",
|
||||
"principalId": "$authenticated",
|
||||
"permission": "ALLOW",
|
||||
"property": "changePassword",
|
||||
"accessType": "EXECUTE"
|
||||
}
|
||||
],
|
||||
"relations": {
|
||||
|
|
|
|||
|
|
@ -117,6 +117,67 @@ describe('users - integration', function() {
|
|||
.set('Authorization', 'unknown-token')
|
||||
.expect(401, done);
|
||||
});
|
||||
|
||||
it('updates the user\'s password', function() {
|
||||
const User = app.models.User;
|
||||
const credentials = {email: 'change@example.com', password: 'pass'};
|
||||
return User.create(credentials)
|
||||
.then(u => {
|
||||
this.user = u;
|
||||
return User.login(credentials);
|
||||
})
|
||||
.then(token => {
|
||||
return this.post('/api/users/change-password')
|
||||
.set('Authorization', token.id)
|
||||
.send({
|
||||
oldPassword: credentials.password,
|
||||
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 change password request', function() {
|
||||
return this.post('/api/users/change-password')
|
||||
.send({
|
||||
oldPassword: 'old password',
|
||||
newPassword: 'new password',
|
||||
})
|
||||
.expect(401);
|
||||
});
|
||||
|
||||
it('injects change password options from remoting context', function() {
|
||||
const User = app.models.User;
|
||||
const credentials = {email: 'inject@example.com', password: 'pass'};
|
||||
|
||||
let injectedOptions;
|
||||
User.observe('before save', (ctx, next) => {
|
||||
injectedOptions = ctx.options;
|
||||
next();
|
||||
});
|
||||
|
||||
return User.create(credentials)
|
||||
.then(u => User.login(credentials))
|
||||
.then(token => {
|
||||
return this.post('/api/users/change-password')
|
||||
.set('Authorization', token.id)
|
||||
.send({
|
||||
oldPassword: credentials.password,
|
||||
newPassword: 'new password',
|
||||
})
|
||||
.expect(204);
|
||||
})
|
||||
.then(() => {
|
||||
expect(injectedOptions).to.have.property('accessToken');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('sub-user', function() {
|
||||
|
|
|
|||
|
|
@ -1267,6 +1267,124 @@ describe('User', function() {
|
|||
});
|
||||
});
|
||||
|
||||
describe('User.changePassword()', () => {
|
||||
let userId, currentPassword;
|
||||
beforeEach(givenUserIdAndPassword);
|
||||
|
||||
it('changes the password - callback-style', done => {
|
||||
User.changePassword(userId, currentPassword, '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.changePassword(userId, currentPassword, '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('changes the password - instance method', () => {
|
||||
validCredentialsUser.changePassword(currentPassword, '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 when current password does not match', () => {
|
||||
return User.changePassword(userId, 'bad password', 'new password').then(
|
||||
success => { throw new Error('changePassword 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: 'INVALID_PASSWORD',
|
||||
statusCode: 400,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
it('fails with 401 for unknown user id', () => {
|
||||
return User.changePassword('unknown-id', 'pass', 'pass').then(
|
||||
success => { throw new Error('changePassword 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.changePassword(userId, currentPassword, 'new', options)
|
||||
.then(() => expect(observedOptions).to.eql([
|
||||
// findById
|
||||
{hook: 'access', testFlag: true},
|
||||
|
||||
// "before save" hook prepareForTokenInvalidation
|
||||
// FIXME(bajtos) the hook should be forwarding the options too!
|
||||
{hook: 'access'},
|
||||
|
||||
// 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 givenUserIdAndPassword() {
|
||||
userId = validCredentialsUser.id;
|
||||
currentPassword = validCredentials.password;
|
||||
}
|
||||
});
|
||||
|
||||
describe('Verification', function() {
|
||||
describe('user.verify(options, fn)', function() {
|
||||
it('Verify a user\'s email address', function(done) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue