From 793d70dca085cdfa7284b5dff52f1318f97a20a9 Mon Sep 17 00:00:00 2001 From: Scott Hulbert Date: Mon, 22 Sep 2014 17:03:29 -0700 Subject: [PATCH 1/3] obey apidoc of optional name, email & prevent privilege changes --- server/routes/api/users.coffee | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/server/routes/api/users.coffee b/server/routes/api/users.coffee index ba81939..a1ea764 100644 --- a/server/routes/api/users.coffee +++ b/server/routes/api/users.coffee @@ -172,17 +172,22 @@ app.route('/users/:userID') return res.status(400).end() if err or not user {password, passwordconfirm, oldpassword} = req.body - delete req.body.password # Only add back after checking below if password and passwordconfirm and oldpassword if password isnt passwordconfirm user.invalidate 'passwordconfirm', 'Your new password and confirmation don’t match.' else if not user.authenticate oldpassword user.invalidate 'oldpassword', 'The provided password is incorrect.' else - # All good - req.body.password = password - - user.set(req.body).save (err, user) -> + user.password = password + + {name, email} = req.body + if name? + user.name = name + + if email? + user.email = email + + user.save (err, user) -> return res.status(400).send err if err res.status(200).send user From 6dcf02771f27f9a681d96370f194a3dfbc0f2dbe Mon Sep 17 00:00:00 2001 From: Scott Hulbert Date: Mon, 22 Sep 2014 18:58:39 -0700 Subject: [PATCH 2/3] bring over david's fix for hidden checkbox, optional roles, safer new user --- client/source/templates/users/edit.hbs | 1 + server/routes/api/users.coffee | 20 +++++++++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/client/source/templates/users/edit.hbs b/client/source/templates/users/edit.hbs index 9857b94..50c5310 100644 --- a/client/source/templates/users/edit.hbs +++ b/client/source/templates/users/edit.hbs @@ -11,6 +11,7 @@ {{input 'password' '' placeholder="New password" type="password"}} {{input 'passwordconfirm' '' placeholder="Confirm new password" type="password"}} + {{hidden 'admin' isAdmin}} {{!-- Viewing someone else's --}} {{else}} diff --git a/server/routes/api/users.coffee b/server/routes/api/users.coffee index a1ea764..8366ff3 100644 --- a/server/routes/api/users.coffee +++ b/server/routes/api/users.coffee @@ -1,6 +1,8 @@ express = require 'express' async = require 'async' crypto = require 'crypto' +_ = require 'underscore' +{isArray} = require 'util' mailer = require '../../lib/mailer' config = require '../../config' @@ -75,6 +77,7 @@ module.exports = app = express() @apiParam {String} name Full name of the user. @apiParam {String} email Email address of the user. @apiParam {String} password Password for the user. Must be between 6-20 characters and include a number. + @apiParam {Array} [roles] Array of roles the user is granted @apiPermission administrator ### @@ -93,13 +96,19 @@ app.route('/users') return res.status(401).end() unless req.user?.hasRole ['administrator'] return req.status(400).end() unless typeof req?.body is 'object' - newUser = new User req.body + data = _.pick req.body, 'name', 'email', 'password' + newUser = new User data + + # optional + if req.body.roles? and isArray(req.body.roles) + newUser.roles = req.body.roles newUser.save (err) -> return res.status(400).send err if err res.status(200).send newUser .get (req, res) -> + # TODO shouldn't we filter passwords and such from here? User.find {}, (err, users) -> res.status(200).send users @@ -125,6 +134,7 @@ app.route('/users') @apiParam {String} id User ID (sent in URL) @apiParam {String} [name] The full name of the user. @apiParam {String} [email] The user’s email address. + @apiParam {Array} [roles] Array of roles the user has @apiPermission administrator @@ -166,7 +176,8 @@ app.route('/users/:userID') res.status(200).end() .put (req, res) -> - return res.status(401).end() unless req.user?.hasRole ['administrator'] or req.user?._id is req.params.userID + adminEditing = req.user?.hasRole ['administrator'] + return res.status(401).end() unless adminEditing or req.user?._id is req.params.userID User.findById req.params.userID, 'passwordDigest', (err, user) -> return res.status(400).end() if err or not user @@ -186,7 +197,10 @@ app.route('/users/:userID') if email? user.email = email - + + if adminEditing and req.body.roles? and isArray(req.body.roles) + user.roles = req.body.roles + user.save (err, user) -> return res.status(400).send err if err res.status(200).send user From 4c28ec7d5331d6e84cda9264470b936d242656b5 Mon Sep 17 00:00:00 2001 From: Scott Hulbert Date: Wed, 24 Sep 2014 19:39:28 -0700 Subject: [PATCH 3/3] use _.pick for optionals too --- server/routes/api/users.coffee | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/server/routes/api/users.coffee b/server/routes/api/users.coffee index 8366ff3..8ce6206 100644 --- a/server/routes/api/users.coffee +++ b/server/routes/api/users.coffee @@ -191,12 +191,7 @@ app.route('/users/:userID') else user.password = password - {name, email} = req.body - if name? - user.name = name - - if email? - user.email = email + user.set _.pick req.body, 'name', 'email' if adminEditing and req.body.roles? and isArray(req.body.roles) user.roles = req.body.roles