Fix "POST /change-password" for multi-user setup

Fix the code extracting current user id from the access token provided
in the HTTP request, to allow only access tokens created by the target
user models to execute the action.

This fixes the following security vulnerability:

* We have two user models, e.g. Admin and Customer

* We have an Admin instance and a Customer instance with the same
  id and the same password.

* The Customer can change Admin's password using their
  regular access token.
This commit is contained in:
Miroslav Bajtoš 2017-10-27 09:47:07 +02:00
parent 4d4070e542
commit 3996f56ab9
No known key found for this signature in database
GPG Key ID: 6F2304BA9361C7E3
2 changed files with 47 additions and 3 deletions

View File

@ -1173,9 +1173,7 @@ module.exports = function(User) {
{
description: 'Change a user\'s password.',
accepts: [
{arg: 'id', type: 'any',
http: ctx => ctx.req.accessToken && ctx.req.accessToken.userId,
},
{arg: 'id', type: 'any', http: getUserIdFromRequestContext},
{arg: 'oldPassword', type: 'string', required: true, http: {source: 'form'}},
{arg: 'newPassword', type: 'string', required: true, http: {source: 'form'}},
{arg: 'options', type: 'object', http: 'optionsFromRequest'},

View File

@ -28,6 +28,7 @@ describe('Multiple users with custom principalType', function() {
// create a local app object that does not share state with other tests
app = loopback({localRegistry: true, loadBuiltinModels: true});
app.set('_verifyAuthModelRelations', false);
app.set('remoting', {errorHandler: {debug: true, log: false}});
app.dataSource('db', {connector: 'memory'});
var userModelOptions = {
@ -672,6 +673,51 @@ describe('Multiple users with custom principalType', function() {
}
});
describe('changePassword', () => {
let token;
beforeEach(givenTokenForOneUser);
it('changes password when the access token belongs to the user', () => {
return supertest(app)
.post('/OneUsers/change-password')
.set('Authorization', token.id)
.send({
oldPassword: commonCredentials.password,
newPassword: 'new-pass',
})
.expect(204)
.then(() => {
return supertest(app)
.post('/OneUsers/login')
.send({email: commonCredentials.email, password: 'new-pass'})
.expect(200);
});
});
it('fails when the access token belongs to a different user mode', () => {
debugger;
logServerErrorsOtherThan(403, app);
return supertest(app)
.post('/AnotherUsers/change-password')
.set('Authorization', token.id)
.send({
oldPassword: commonCredentials.password,
newPassword: 'new-pass',
})
.expect(403)
.then(() => {
return supertest(app)
.post('/AnotherUsers/login')
.send(commonCredentials)
.expect(200);
});
});
function givenTokenForOneUser() {
return OneUser.login(commonCredentials).then(t => token = t);
}
});
// helpers
function createUserModel(app, name, options) {
var model = app.registry.createModel(Object.assign({name: name}, options));