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

Sanitise user #1417

Merged
merged 1 commit into from
Aug 31, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions modules/users/server/controllers/admin.server.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');

/**
* Update user details
Expand Down Expand Up @@ -101,5 +102,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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Wuntenn Can you explain the advantages for us to use our own passport verification here, as described in the mock example? How does this differ from our current Strategy implementation?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mleanos I don't think we need to create any mock. This looks just well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. I was questioning the TODO note.

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