Merge pull request #3666 from strongloop/fix/multi-user-reset-password

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

common/models/user.js

@@ -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) {

test/authorization-scopes.test.js

@@ -3,6 +3,10 @@
 const loopback = require('../');
 const supertest = require('supertest');
 const strongErrorHandler = require('strong-error-handler');
+const loggers = require('./helpers/error-loggers');
+
+const logAllServerErrors = loggers.logAllServerErrors;
+const logServerErrorsOtherThan = loggers.logServerErrorsOtherThan;
 
 describe('Authorization scopes', () => {
   const CUSTOM_SCOPE = 'read:custom';
@@ -15,28 +19,28 @@ describe('Authorization scopes', () => {
   beforeEach(givenScopedToken);
 
   it('denies regular token to invoke custom-scoped method', () => {
-    logServerErrorsOtherThan(401);
+    logServerErrorsOtherThan(401, app);
     return request.get('/users/scoped')
       .set('Authorization', regularToken.id)
       .expect(401);
   });
 
   it('allows regular tokens to invoke default-scoped method', () => {
-    logAllServerErrors();
+    logAllServerErrors(app);
     return request.get('/users/' + testUser.id)
       .set('Authorization', regularToken.id)
       .expect(200);
   });
 
   it('allows scoped token to invoke custom-scoped method', () => {
-    logAllServerErrors();
+    logAllServerErrors(app);
     return request.get('/users/scoped')
       .set('Authorization', scopedToken.id)
       .expect(204);
   });
 
   it('denies scoped token to invoke default-scoped method', () => {
-    logServerErrorsOtherThan(401);
+    logServerErrorsOtherThan(401, app);
     return request.get('/users/' + testUser.id)
       .set('Authorization', scopedToken.id)
       .expect(401);
@@ -45,7 +49,7 @@ describe('Authorization scopes', () => {
   describe('token granted both default and custom scope', () => {
     beforeEach('given token with default and custom scope',
       () => givenScopedToken(['DEFAULT', CUSTOM_SCOPE]));
-    beforeEach(logAllServerErrors);
+    beforeEach(() => logAllServerErrors(app));
 
     it('allows invocation of default-scoped method', () => {
       return request.get('/users/' + testUser.id)
@@ -116,19 +120,4 @@ describe('Authorization scopes', () => {
     return testUser.accessTokens.create({ttl: 60, scopes})
       .then(t => scopedToken = t);
   }
-
-  function logAllServerErrors() {
-    logServerErrorsOtherThan(-1);
-  }
-
-  function logServerErrorsOtherThan(statusCode) {
-    app.use((err, req, res, next) => {
-      if ((err.statusCode || 500) !== statusCode) {
-        console.log('Unhandled error for request %s %s: %s',
-          req.method, req.url, err.stack || err);
-      }
-      res.statusCode = err.statusCode || 500;
-      res.json(err);
-    });
-  }
 });

test/helpers/error-loggers.js

@@ -0,0 +1,21 @@
+// Copyright IBM Corp. 2015,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';
+
+exports.logAllServerErrors = function(app) {
+  exports.logServerErrorsOtherThan(-1, app);
+};
+
+exports.logServerErrorsOtherThan = function(statusCode, app) {
+  app.use((err, req, res, next) => {
+    if ((err.statusCode || 500) !== statusCode) {
+      console.log('Unhandled error for request %s %s: %s',
+        req.method, req.url, err.stack || err);
+    }
+    res.statusCode = err.statusCode || 500;
+    res.json(err);
+  });
+};

test/multiple-user-principal-types.test.js

@@ -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));