Fix "POST /reset-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:

* A UserA with id 1 (for example), requires a resetToken1

* A UserB with the same id requires a resetToken2.

* Using resetToken2, use the UserAs/reset-password endpoint and change
  the password of UserA and/or vice-versa.
This commit is contained in:
Miroslav Bajtoš 2017-10-19 13:29:08 +02:00
parent 4ebc517a78
commit 0a2a45512c
No known key found for this signature in database
GPG Key ID: 6F2304BA9361C7E3
2 changed files with 65 additions and 3 deletions

View File

@ -1192,9 +1192,7 @@ module.exports = function(User) {
{
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: 'id', type: 'any', http: getUserIdFromRequestContext},
{arg: 'newPassword', type: 'string', required: true, http: {source: 'form'}},
{arg: 'options', type: 'object', http: 'optionsFromRequest'},
],
@ -1203,6 +1201,23 @@ module.exports = function(User) {
}
);
function getUserIdFromRequestContext(ctx) {
const token = ctx.req.accessToken;
if (!token) return;
const hasPrincipalType = 'principalType' in token;
if (hasPrincipalType && token.principalType !== UserModel.modelName) {
// We have multiple user models related to the same access token model
// and the token used to authorize reset-password request was created
// for a different user model.
const err = new Error(g.f('Access Denied'));
err.statusCode = 403;
throw err;
}
return token.userId;
}
UserModel.afterRemote('confirm', function(ctx, inst, next) {
if (ctx.args.redirect !== undefined) {
if (!ctx.res) {

View File

@ -12,6 +12,10 @@ var extend = require('util')._extend;
var AccessContext = ctx.AccessContext;
var Principal = ctx.Principal;
var Promise = require('bluebird');
const waitForEvent = require('./helpers/wait-for-event');
const supertest = require('supertest');
const loggers = require('./helpers/error-loggers');
const logServerErrorsOtherThan = loggers.logServerErrorsOtherThan;
describe('Multiple users with custom principalType', function() {
this.timeout(10000);
@ -55,6 +59,7 @@ describe('Multiple users with custom principalType', function() {
app.enableAuth({dataSource: 'db'});
app.use(loopback.token({model: AccessToken}));
app.use(loopback.rest());
// create one user per user model to use them throughout the tests
return Promise.all([
@ -625,6 +630,48 @@ describe('Multiple users with custom principalType', function() {
});
});
describe('setPassword', () => {
let resetToken;
beforeEach(givenResetPasswordTokenForOneUser);
it('sets password when the access token belongs to the user', () => {
return supertest(app)
.post('/OneUsers/reset-password')
.set('Authorization', resetToken.id)
.send({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', () => {
logServerErrorsOtherThan(403, app);
return supertest(app)
.post('/AnotherUsers/reset-password')
.set('Authorization', resetToken.id)
.send({newPassword: 'new-pass'})
.expect(403)
.then(() => {
return supertest(app)
.post('/AnotherUsers/login')
.send(commonCredentials)
.expect(200);
});
});
function givenResetPasswordTokenForOneUser() {
return Promise.all([
OneUser.resetPassword({email: commonCredentials.email}),
waitForEvent(OneUser, 'resetPasswordRequest'),
])
.spread((reset, info) => resetToken = info.accessToken);
}
});
// helpers
function createUserModel(app, name, options) {
var model = app.registry.createModel(Object.assign({name: name}, options));