diff --git a/routes/api/middleware.js b/routes/api/middleware.js index 8094b46..d2aec75 100644 --- a/routes/api/middleware.js +++ b/routes/api/middleware.js @@ -1,12 +1,12 @@ 'use strict'; var errors = require( 'errors' ); -var debug = require('debug')('dpac:api.middleware'); +var debug = require( 'debug' )( 'dpac:api.middleware' ); exports.initAPI = function initAPI( req, res, next ){ - debug('initAPI'); + debug( 'initAPI' ); res.apiResponse = function( status ){ if( req.query.callback ){ res.jsonp( status ); @@ -30,7 +30,7 @@ exports.initAPI = function initAPI( req, exports.requireUser = function( req, res, next ){ - debug('requireUser'); + debug( 'requireUser' ); if( !req.user ){ res.apiError( new errors.Http401Error() ); }else{ @@ -41,16 +41,28 @@ exports.requireUser = function( req, exports.methodNotAllowed = function( req, res, next ){ - res.apiError( new errors.Http406Error() ); + return res.apiError( new errors.Http406Error() ); }; -exports.requireAdmin = function(req, - res, - next){ - debug('requireAdmin'); - if( !req.user.isAdmin){ - res.apiError( new errors.Http401Error() ); +exports.requireAdmin = function( req, + res, + next ){ + debug( 'requireAdmin' ); + if( !req.user.isAdmin ){ + return res.apiError( new errors.Http401Error() ); }else{ next(); } }; + +exports.handleError = function( err, + req, + res, + next ){ + console.error( err ); + if( (err.name && 'CastError' === err.name) && (err.path && '_id' === err.path ) ){ + return res.apiError( new errors.Http404Error() ); + }else{ + return res.apiError( new errors.Http500Error() ); + } +}; diff --git a/routes/api/users/controller.js b/routes/api/users/controller.js index e504433..a0f560c 100644 --- a/routes/api/users/controller.js +++ b/routes/api/users/controller.js @@ -2,45 +2,58 @@ var keystone = require( 'keystone' ), async = require( 'async' ); var errors = require( 'errors' ); -var debug =require('debug')('dpac:api.users.controller'); +var debug = require( 'debug' )( 'dpac:api.users.controller' ); var User = keystone.list( 'User' ).model; -module.exports.list = function(req, - res, - next){ - debug('list'); - User.find().exec(function(err, users){ - var results={}; - users.forEach(function(user){ +module.exports.list = function( req, + res, + next ){ + debug( 'list' ); + User.find().exec( function( err, + users ){ + if( err ){ + return next( err ); + } + if( !users ){ + return res.apiError( new errors.Http404Error() ); + } + var results = { }; + users.forEach( function( user ){ results[user.id] = user; - }); - res.apiResponse(users); - }); + } ); + res.apiResponse( users ); + } ); }; module.exports.retrieve = function( req, res, next ){ - debug('retrieve'); - User.findById( res.locals.user.id, function( err, - user ){ - if( err || !user ){ + debug( 'retrieve' ); + User.findById( res.locals.user.id ).exec( function( err, + user ){ + if( err ){ + return next( err ); + } + if( !user ){ return res.apiError( new errors.Http404Error() ); } - return res.apiResponse( user.toJSON() ); + return res.apiResponse( user ); } ); }; module.exports.update = function( req, res, next ){ - debug('update'); - User.findOneAndUpdate( - { _id : res.locals.user.id }, - req.body, - function( err, - user ){ - return res.apiResponse( user.toJSON() ); + debug( 'update' ); + User.findByIdAndUpdate( res.locals.user.id, req.body ).exec( function( err, + user ){ + if( err ){ + return next( err ); + } + if( !user ){ + return res.apiError( new errors.Http404Error() ); + } + return res.apiResponse( user ); } ); }; diff --git a/routes/index.js b/routes/index.js index 6a26bc9..1a0b9ab 100644 --- a/routes/index.js +++ b/routes/index.js @@ -56,6 +56,9 @@ exports = module.exports = function( app ){ app.patch( '/api/users/:id', api.users.controller.update ); app.all( '/api/users*', api.middleware.methodNotAllowed ); + + app.all( '/api*', api.middleware.handleError ); + // NOTE: To protect a route so that only admins can see it, use the requireUser middleware: // app.get('/protected', middleware.requireUser, routes.views.protected);