From 54ae7dc564a4d9daa2d896623e22fae856b8a375 Mon Sep 17 00:00:00 2001
From: Daron Jones <daronjones@hotmail.com>
Date: Wed, 31 Aug 2016 20:50:23 +0100
Subject: [PATCH] feat(users): prevent route leaking access token (#1417)

The test for authentication use a route /api/users/me. This should probably be upgraded to use
a proper passport mock.

In the meanwhile this should make the returned user object safer - using code from core.

Fixes n/a
---
 .../controllers/admin.server.controller.js    |  4 ++--
 .../users/users.profile.server.controller.js  | 23 +++++++++++++++++--
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/modules/users/server/controllers/admin.server.controller.js b/modules/users/server/controllers/admin.server.controller.js
index 46fc41d4b9..3747a06e68 100644
--- a/modules/users/server/controllers/admin.server.controller.js
+++ b/modules/users/server/controllers/admin.server.controller.js
@@ -59,7 +59,7 @@ exports.delete = function (req, res) {
  * List of Users
  */
 exports.list = function (req, res) {
-  User.find({}, '-salt -password').sort('-created').populate('user', 'displayName').exec(function (err, users) {
+  User.find({}, '-salt -password -providerData').sort('-created').populate('user', 'displayName').exec(function (err, users) {
     if (err) {
       return res.status(400).send({
         message: errorHandler.getErrorMessage(err)
@@ -80,7 +80,7 @@ exports.userByID = function (req, res, next, id) {
     });
   }
 
-  User.findById(id, '-salt -password').exec(function (err, user) {
+  User.findById(id, '-salt -password -providerData').exec(function (err, user) {
     if (err) {
       return next(err);
     } else if (!user) {
diff --git a/modules/users/server/controllers/users/users.profile.server.controller.js b/modules/users/server/controllers/users/users.profile.server.controller.js
index 7f32894d55..99f8e7bc99 100644
--- a/modules/users/server/controllers/users/users.profile.server.controller.js
+++ b/modules/users/server/controllers/users/users.profile.server.controller.js
@@ -10,7 +10,8 @@ var _ = require('lodash'),
   mongoose = require('mongoose'),
   multer = require('multer'),
   config = require(path.resolve('./config/config')),
-  User = mongoose.model('User');
+  User = mongoose.model('User'),
+  validator = require('validator');
 
 var whitelistedFields = ['firstName', 'lastName', 'email', 'username'];
 
@@ -141,5 +142,23 @@ exports.changeProfilePicture = function (req, res) {
  * Send User
  */
 exports.me = function (req, res) {
-  res.json(req.user || null);
+  // Sanitize the user - short term solution. Copied from core.server.controller.js
+  // TODO create proper passport mock: See https://gist.github.com/mweibel/5219403
+  var safeUserObject = null;
+  if (req.user) {
+    safeUserObject = {
+      displayName: validator.escape(req.user.displayName),
+      provider: validator.escape(req.user.provider),
+      username: validator.escape(req.user.username),
+      created: req.user.created.toString(),
+      roles: req.user.roles,
+      profileImageURL: req.user.profileImageURL,
+      email: validator.escape(req.user.email),
+      lastName: validator.escape(req.user.lastName),
+      firstName: validator.escape(req.user.firstName),
+      additionalProvidersData: req.user.additionalProvidersData
+    };
+  }
+
+  res.json(safeUserObject || null);
 };