From 0afb2e6ec93b251730d5a7a55b98053c4415ab5b Mon Sep 17 00:00:00 2001 From: Liran Tal Date: Sun, 12 Jan 2014 23:55:34 +0200 Subject: [PATCH 1/2] refactoring so that app routes can be maintained more easily from app/ directory (which makes more sense) --- app/routes/articles.js | 16 ++++++++++++++++ app/routes/index.js | 9 +++++++++ config/routes.js => app/routes/users.js | 18 +----------------- server.js | 17 ++++++++++++++++- 4 files changed, 42 insertions(+), 18 deletions(-) create mode 100644 app/routes/articles.js create mode 100644 app/routes/index.js rename config/routes.js => app/routes/users.js (73%) mode change 100755 => 100644 diff --git a/app/routes/articles.js b/app/routes/articles.js new file mode 100644 index 0000000000..7464075821 --- /dev/null +++ b/app/routes/articles.js @@ -0,0 +1,16 @@ +'use strict'; + +module.exports = function(app, passport, auth) { + + // Article Routes + var articles = require('../controllers/articles'); + app.get('/articles', articles.all); + app.post('/articles', auth.requiresLogin, articles.create); + app.get('/articles/:articleId', articles.show); + app.put('/articles/:articleId', auth.requiresLogin, auth.article.hasAuthorization, articles.update); + app.del('/articles/:articleId', auth.requiresLogin, auth.article.hasAuthorization, articles.destroy); + + // Finish with setting up the articleId param + app.param('articleId', articles.article); + +}; diff --git a/app/routes/index.js b/app/routes/index.js new file mode 100644 index 0000000000..f0e8c23e62 --- /dev/null +++ b/app/routes/index.js @@ -0,0 +1,9 @@ +'use strict'; + +module.exports = function(app, passport, auth) { + + // Home route + var index = require('../controllers/index'); + app.get('/', index.render); + +}; diff --git a/config/routes.js b/app/routes/users.js old mode 100755 new mode 100644 similarity index 73% rename from config/routes.js rename to app/routes/users.js index 4a3e6f52ba..27fa716773 --- a/config/routes.js +++ b/app/routes/users.js @@ -3,7 +3,7 @@ module.exports = function(app, passport, auth) { // User Routes - var users = require('../app/controllers/users'); + var users = require('../controllers/users'); app.get('/signin', users.signin); app.get('/signup', users.signup); app.get('/signout', users.signout); @@ -62,20 +62,4 @@ module.exports = function(app, passport, auth) { failureRedirect: '/signin' }), users.authCallback); - - // Article Routes - var articles = require('../app/controllers/articles'); - app.get('/articles', articles.all); - app.post('/articles', auth.requiresLogin, articles.create); - app.get('/articles/:articleId', articles.show); - app.put('/articles/:articleId', auth.requiresLogin, auth.article.hasAuthorization, articles.update); - app.del('/articles/:articleId', auth.requiresLogin, auth.article.hasAuthorization, articles.destroy); - - // Finish with setting up the articleId param - app.param('articleId', articles.article); - - // Home route - var index = require('../app/controllers/index'); - app.get('/', index.render); - }; diff --git a/server.js b/server.js index 1cb926fe12..7a78867d57 100755 --- a/server.js +++ b/server.js @@ -51,7 +51,22 @@ var app = express(); require('./config/express')(app, passport, db); // Bootstrap routes -require('./config/routes')(app, passport, auth); +var routes_path = __dirname + '/app/routes'; +var walk = function(path) { + fs.readdirSync(path).forEach(function(file) { + var newPath = path + '/' + file; + var stat = fs.statSync(newPath); + if (stat.isFile()) { + if (/(.*)\.(js$|coffee$)/.test(file)) { + require(newPath)(app, passport, auth); + } + } else if (stat.isDirectory()) { + walk(newPath); + } + }); +}; +walk(routes_path); + // Start the app by listening on var port = process.env.PORT || config.port; From dea044c2e1e1bc3d0e4d7eb62485641f4e7079f0 Mon Sep 17 00:00:00 2001 From: Liran Tal Date: Mon, 13 Jan 2014 00:42:53 +0200 Subject: [PATCH 2/2] further refactoring of the route middlewares so they can be easily shared amongst app routes --- app/routes/articles.js | 24 ++++++++++++----- app/routes/index.js | 2 +- app/routes/middlewares/authorization.js | 11 ++++++++ app/routes/users.js | 17 +++++++++--- config/middlewares/authorization.js | 35 ------------------------- server.js | 8 +++--- 6 files changed, 47 insertions(+), 50 deletions(-) create mode 100644 app/routes/middlewares/authorization.js delete mode 100755 config/middlewares/authorization.js diff --git a/app/routes/articles.js b/app/routes/articles.js index 7464075821..4c68a4a959 100644 --- a/app/routes/articles.js +++ b/app/routes/articles.js @@ -1,14 +1,24 @@ 'use strict'; -module.exports = function(app, passport, auth) { - - // Article Routes - var articles = require('../controllers/articles'); +// Articles routes use articles controller +var articles = require('../controllers/articles'); +var authorization = require('./middlewares/authorization'); + +// Article authorization helpers +var hasAuthorization = function(req, res, next) { + if (req.article.user.id != req.user.id) { + return res.send(401, 'User is not authorized'); + } + next(); +} + +module.exports = function(app, passport) { + app.get('/articles', articles.all); - app.post('/articles', auth.requiresLogin, articles.create); + app.post('/articles', authorization.requiresLogin, articles.create); app.get('/articles/:articleId', articles.show); - app.put('/articles/:articleId', auth.requiresLogin, auth.article.hasAuthorization, articles.update); - app.del('/articles/:articleId', auth.requiresLogin, auth.article.hasAuthorization, articles.destroy); + app.put('/articles/:articleId', authorization.requiresLogin, hasAuthorization, articles.update); + app.del('/articles/:articleId', authorization.requiresLogin, hasAuthorization, articles.destroy); // Finish with setting up the articleId param app.param('articleId', articles.article); diff --git a/app/routes/index.js b/app/routes/index.js index f0e8c23e62..688ea14996 100644 --- a/app/routes/index.js +++ b/app/routes/index.js @@ -1,6 +1,6 @@ 'use strict'; -module.exports = function(app, passport, auth) { +module.exports = function(app, passport) { // Home route var index = require('../controllers/index'); diff --git a/app/routes/middlewares/authorization.js b/app/routes/middlewares/authorization.js new file mode 100644 index 0000000000..eea27c54ef --- /dev/null +++ b/app/routes/middlewares/authorization.js @@ -0,0 +1,11 @@ +'use strict'; + +/** + * Generic require login routing middleware + */ +exports.requiresLogin = function(req, res, next) { + if (!req.isAuthenticated()) { + return res.send(401, 'User is not authorized'); + } + next(); +}; \ No newline at end of file diff --git a/app/routes/users.js b/app/routes/users.js index 27fa716773..0cc59b42fb 100644 --- a/app/routes/users.js +++ b/app/routes/users.js @@ -1,9 +1,18 @@ 'use strict'; -module.exports = function(app, passport, auth) { - - // User Routes - var users = require('../controllers/users'); +// User routes use users controller +var users = require('../controllers/users'); + +// User authorization helpers +var hasAuthorization = function(req, res, next) { + if (req.profile.id != req.user.id) { + return res.send(401, 'User is not authorized'); + } + next(); +} + +module.exports = function(app, passport) { + app.get('/signin', users.signin); app.get('/signup', users.signup); app.get('/signout', users.signout); diff --git a/config/middlewares/authorization.js b/config/middlewares/authorization.js deleted file mode 100755 index 8bbb0b2a38..0000000000 --- a/config/middlewares/authorization.js +++ /dev/null @@ -1,35 +0,0 @@ -'use strict'; - -/** - * Generic require login routing middleware - */ -exports.requiresLogin = function(req, res, next) { - if (!req.isAuthenticated()) { - return res.send(401, 'User is not authorized'); - } - next(); -}; - -/** - * User authorizations routing middleware - */ -exports.user = { - hasAuthorization: function(req, res, next) { - if (req.profile.id != req.user.id) { - return res.send(401, 'User is not authorized'); - } - next(); - } -}; - -/** - * Article authorizations routing middleware - */ -exports.article = { - hasAuthorization: function(req, res, next) { - if (req.article.user.id != req.user.id) { - return res.send(401, 'User is not authorized'); - } - next(); - } -}; \ No newline at end of file diff --git a/server.js b/server.js index 7a78867d57..279d02ca83 100755 --- a/server.js +++ b/server.js @@ -19,7 +19,6 @@ process.env.NODE_ENV = process.env.NODE_ENV || 'development'; // Initializing system variables var config = require('./config/config'), - auth = require('./config/middlewares/authorization'), mongoose = require('mongoose'); // Bootstrap db connection @@ -58,9 +57,12 @@ var walk = function(path) { var stat = fs.statSync(newPath); if (stat.isFile()) { if (/(.*)\.(js$|coffee$)/.test(file)) { - require(newPath)(app, passport, auth); + require(newPath)(app, passport); } - } else if (stat.isDirectory()) { + // We skip the app/routes/middlewares directory as it is meant to be + // used and shared by routes as further middlewares and is not a + // route by itself + } else if (stat.isDirectory() && file !== 'middlewares') { walk(newPath); } });