Skip to content
This repository was archived by the owner on Aug 30, 2021. It is now read-only.

Commit 54ae7dc

Browse files
Wuntennlirantal
authored andcommitted
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
1 parent 55525bd commit 54ae7dc

File tree

2 files changed

+23
-4
lines changed

2 files changed

+23
-4
lines changed

modules/users/server/controllers/admin.server.controller.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ exports.delete = function (req, res) {
5959
* List of Users
6060
*/
6161
exports.list = function (req, res) {
62-
User.find({}, '-salt -password').sort('-created').populate('user', 'displayName').exec(function (err, users) {
62+
User.find({}, '-salt -password -providerData').sort('-created').populate('user', 'displayName').exec(function (err, users) {
6363
if (err) {
6464
return res.status(400).send({
6565
message: errorHandler.getErrorMessage(err)
@@ -80,7 +80,7 @@ exports.userByID = function (req, res, next, id) {
8080
});
8181
}
8282

83-
User.findById(id, '-salt -password').exec(function (err, user) {
83+
User.findById(id, '-salt -password -providerData').exec(function (err, user) {
8484
if (err) {
8585
return next(err);
8686
} else if (!user) {

modules/users/server/controllers/users/users.profile.server.controller.js

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ var _ = require('lodash'),
1010
mongoose = require('mongoose'),
1111
multer = require('multer'),
1212
config = require(path.resolve('./config/config')),
13-
User = mongoose.model('User');
13+
User = mongoose.model('User'),
14+
validator = require('validator');
1415

1516
var whitelistedFields = ['firstName', 'lastName', 'email', 'username'];
1617

@@ -141,5 +142,23 @@ exports.changeProfilePicture = function (req, res) {
141142
* Send User
142143
*/
143144
exports.me = function (req, res) {
144-
res.json(req.user || null);
145+
// Sanitize the user - short term solution. Copied from core.server.controller.js
146+
// TODO create proper passport mock: See https://gist.github.com/mweibel/5219403
147+
var safeUserObject = null;
148+
if (req.user) {
149+
safeUserObject = {
150+
displayName: validator.escape(req.user.displayName),
151+
provider: validator.escape(req.user.provider),
152+
username: validator.escape(req.user.username),
153+
created: req.user.created.toString(),
154+
roles: req.user.roles,
155+
profileImageURL: req.user.profileImageURL,
156+
email: validator.escape(req.user.email),
157+
lastName: validator.escape(req.user.lastName),
158+
firstName: validator.escape(req.user.firstName),
159+
additionalProvidersData: req.user.additionalProvidersData
160+
};
161+
}
162+
163+
res.json(safeUserObject || null);
145164
};

0 commit comments

Comments
 (0)