From ddffbc699d0cc201a16ec25419c8ba7c1dc3a2e5 Mon Sep 17 00:00:00 2001 From: ivaosthu Date: Thu, 16 Nov 2017 15:13:39 +0100 Subject: [PATCH 1/3] Add cookie-session dependency --- lib/middleware/session-middleware.js | 8 ++++++++ package.json | 1 + yarn.lock | 20 ++++++++++++++++++++ 3 files changed, 29 insertions(+) create mode 100644 lib/middleware/session-middleware.js diff --git a/lib/middleware/session-middleware.js b/lib/middleware/session-middleware.js new file mode 100644 index 0000000000..f33e63cad3 --- /dev/null +++ b/lib/middleware/session-middleware.js @@ -0,0 +1,8 @@ +'use strict'; + +const cookieSession = require('cookie-session'); + +module.exports = config => { + config.a = 1; + return cookieSession(config.field); +}; diff --git a/package.json b/package.json index d128c058d6..7daacc0625 100644 --- a/package.json +++ b/package.json @@ -60,6 +60,7 @@ "body-parser": "^1.18.2", "commander": "^2.9.0", "cookie-parser": "^1.4.3", + "cookie-session": "^2.0.0-beta.3", "db-migrate": "0.10.0-beta.24", "db-migrate-pg": "^0.2.4", "deep-diff": "^0.3.3", diff --git a/yarn.lock b/yarn.lock index 5dc8e1f133..17c7a3880a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1189,6 +1189,15 @@ cookie-parser@^1.4.3: cookie "0.3.1" cookie-signature "1.0.6" +cookie-session@^2.0.0-beta.3: + version "2.0.0-beta.3" + resolved "https://registry.yarnpkg.com/cookie-session/-/cookie-session-2.0.0-beta.3.tgz#4e446bd9f85bd7e27d3e226f4e99af12011a4386" + dependencies: + cookies "0.7.1" + debug "3.1.0" + on-headers "~1.0.1" + safe-buffer "5.1.1" + cookie-signature@1.0.6: version "1.0.6" resolved "https://registry.yarnpkg.com/cookie-signature/-/cookie-signature-1.0.6.tgz#e303a882b342cc3ee8ca513a79999734dab3ae2c" @@ -1201,6 +1210,13 @@ cookiejar@^2.1.0: version "2.1.1" resolved "https://registry.yarnpkg.com/cookiejar/-/cookiejar-2.1.1.tgz#41ad57b1b555951ec171412a81942b1e8200d34a" +cookies@0.7.1: + version "0.7.1" + resolved "https://registry.yarnpkg.com/cookies/-/cookies-0.7.1.tgz#7c8a615f5481c61ab9f16c833731bcb8f663b99b" + dependencies: + depd "~1.1.1" + keygrip "~1.0.2" + core-assert@^0.2.0: version "0.2.1" resolved "https://registry.yarnpkg.com/core-assert/-/core-assert-0.2.1.tgz#f85e2cf9bfed28f773cc8b3fa5c5b69bdc02fe3f" @@ -3068,6 +3084,10 @@ jsprim@^1.2.2: json-schema "0.2.3" verror "1.10.0" +keygrip@~1.0.2: + version "1.0.2" + resolved "https://registry.yarnpkg.com/keygrip/-/keygrip-1.0.2.tgz#ad3297c557069dea8bcfe7a4fa491b75c5ddeb91" + kind-of@^3.0.2: version "3.2.2" resolved "https://registry.yarnpkg.com/kind-of/-/kind-of-3.2.2.tgz#31ea21a734bab9bbb0f32466d893aea51e4a3c64" From 9fdb948c691a4b245cdeda24ed2456418055d65f Mon Sep 17 00:00:00 2001 From: ivaosthu Date: Thu, 16 Nov 2017 15:41:33 +0100 Subject: [PATCH 2/3] Implement cookie-session support. Sessions will be required to solve admin-auth. I also refactored a few middlewares into seperate files to make the code easier to read. closes #262 --- lib/app.js | 51 ++++++---------------------- lib/middleware/request-logger.js | 12 +++++++ lib/middleware/response-time.js | 16 +++++++++ lib/middleware/session-middleware.js | 8 ----- lib/middleware/session.js | 11 ++++++ lib/middleware/validator.js | 11 ++++++ lib/options.js | 3 ++ lib/routes/admin-api/index.js | 4 +-- 8 files changed, 65 insertions(+), 51 deletions(-) create mode 100644 lib/middleware/request-logger.js create mode 100644 lib/middleware/response-time.js delete mode 100644 lib/middleware/session-middleware.js create mode 100644 lib/middleware/session.js create mode 100644 lib/middleware/validator.js diff --git a/lib/app.js b/lib/app.js index 24286f3f3c..0be1fe787a 100644 --- a/lib/app.js +++ b/lib/app.js @@ -4,20 +4,18 @@ const express = require('express'); const favicon = require('serve-favicon'); const bodyParser = require('body-parser'); const cookieParser = require('cookie-parser'); -const validator = require('express-validator'); -const responseTime = require('response-time'); -const logger = require('./logger')('app.js'); const routes = require('./routes'); const path = require('path'); const errorHandler = require('errorhandler'); - -const { REQUEST_TIME } = require('./events'); +const unleashSession = require('./middleware/session'); +const responseTime = require('./middleware/response-time'); +const requestLogger = require('./middleware/request-logger'); +const validator = require('./middleware/validator'); module.exports = function(config) { const app = express(); const baseUriPath = config.baseUriPath || ''; - const publicFolder = config.publicFolder; app.set('trust proxy'); app.disable('x-powered-by'); @@ -29,42 +27,15 @@ module.exports = function(config) { } app.use(cookieParser()); - - if (publicFolder) { - app.use(favicon(path.join(publicFolder, 'favicon.ico'))); - } - - app.use( - responseTime((req, res, time) => { - const timingInfo = { - path: req.baseUrl, - method: req.method, - statusCode: res.statusCode, - time, - }; - config.eventBus.emit(REQUEST_TIME, timingInfo); - }) - ); - - app.use( - validator({ - customValidators: { - isUrlFirendlyName: input => encodeURIComponent(input) === input, - }, - }) - ); - - if (publicFolder) { - app.use(baseUriPath, express.static(publicFolder)); - } - app.use(bodyParser.json({ strict: false })); + app.use(unleashSession(config)); + app.use(responseTime(config)); + app.use(requestLogger(config)); + app.use(validator(config)); - if (config.enableRequestLogger) { - app.use((req, res, next) => { - next(); - logger.info(`${res.statusCode} ${req.method} ${req.baseUrl}`); - }); + if (config.publicFolder) { + app.use(favicon(path.join(config.publicFolder, 'favicon.ico'))); + app.use(baseUriPath, express.static(config.publicFolder)); } if (typeof config.preRouterHook === 'function') { diff --git a/lib/middleware/request-logger.js b/lib/middleware/request-logger.js new file mode 100644 index 0000000000..10ac27644a --- /dev/null +++ b/lib/middleware/request-logger.js @@ -0,0 +1,12 @@ +'use strict'; + +const logger = require('../logger')('HTTP'); + +module.exports = function(config) { + return (req, res, next) => { + next(); + if (config.enableRequestLogger) { + logger.info(`${res.statusCode} ${req.method} ${req.baseUrl}`); + } + }; +}; diff --git a/lib/middleware/response-time.js b/lib/middleware/response-time.js new file mode 100644 index 0000000000..4b4ee140bd --- /dev/null +++ b/lib/middleware/response-time.js @@ -0,0 +1,16 @@ +'use strict'; + +const responseTime = require('response-time'); +const { REQUEST_TIME } = require('../events'); + +module.exports = function(config) { + return responseTime((req, res, time) => { + const timingInfo = { + path: req.baseUrl, + method: req.method, + statusCode: res.statusCode, + time, + }; + config.eventBus.emit(REQUEST_TIME, timingInfo); + }); +}; diff --git a/lib/middleware/session-middleware.js b/lib/middleware/session-middleware.js deleted file mode 100644 index f33e63cad3..0000000000 --- a/lib/middleware/session-middleware.js +++ /dev/null @@ -1,8 +0,0 @@ -'use strict'; - -const cookieSession = require('cookie-session'); - -module.exports = config => { - config.a = 1; - return cookieSession(config.field); -}; diff --git a/lib/middleware/session.js b/lib/middleware/session.js new file mode 100644 index 0000000000..70126245e3 --- /dev/null +++ b/lib/middleware/session.js @@ -0,0 +1,11 @@ +'use strict'; + +const cookieSession = require('cookie-session'); + +module.exports = function(config) { + return cookieSession({ + name: 'unleash-session', + keys: [config.secret], + maxAge: config.sessionAge, + }); +}; diff --git a/lib/middleware/validator.js b/lib/middleware/validator.js new file mode 100644 index 0000000000..c66b05328f --- /dev/null +++ b/lib/middleware/validator.js @@ -0,0 +1,11 @@ +'use strict'; + +const validator = require('express-validator'); + +module.exports = function() { + return validator({ + customValidators: { + isUrlFirendlyName: input => encodeURIComponent(input) === input, + }, + }); +}; diff --git a/lib/options.js b/lib/options.js index ae2aba1e4b..3e5f7deee7 100644 --- a/lib/options.js +++ b/lib/options.js @@ -3,6 +3,7 @@ const { publicFolder } = require('unleash-frontend'); const isDev = () => process.env.NODE_ENV === 'development'; +const THIRTY_DAYS = 30 * 24 * 60 * 60 * 1000; const DEFAULT_OPTIONS = { databaseUrl: process.env.DATABASE_URL, @@ -12,6 +13,8 @@ const DEFAULT_OPTIONS = { enableLegacyRoutes: true, publicFolder, enableRequestLogger: isDev(), + secret: 'UNLEASH-SECRET', + sessionAge: THIRTY_DAYS, }; module.exports = { diff --git a/lib/routes/admin-api/index.js b/lib/routes/admin-api/index.js index 72b28e07be..7fe231f4e3 100644 --- a/lib/routes/admin-api/index.js +++ b/lib/routes/admin-api/index.js @@ -24,9 +24,7 @@ exports.apiDef = apiDef; exports.router = config => { const router = Router(); - router.get('/', (req, res) => { - res.json(apiDef); - }); + router.get('/', (req, res) => res.json(apiDef)); router.use('/features', features.router(config)); router.use('/archive', featureArchive.router(config)); From 8c02cc4949848a3e802364c02ed182ba7b529107 Mon Sep 17 00:00:00 2001 From: ivaosthu Date: Thu, 16 Nov 2017 16:07:27 +0100 Subject: [PATCH 3/3] Fix typo in custom validator --- lib/middleware/validator.js | 2 +- lib/routes/admin-api/feature.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/middleware/validator.js b/lib/middleware/validator.js index c66b05328f..0cb71c33d3 100644 --- a/lib/middleware/validator.js +++ b/lib/middleware/validator.js @@ -5,7 +5,7 @@ const validator = require('express-validator'); module.exports = function() { return validator({ customValidators: { - isUrlFirendlyName: input => encodeURIComponent(input) === input, + isUrlFriendlyName: input => encodeURIComponent(input) === input, }, }); }; diff --git a/lib/routes/admin-api/feature.js b/lib/routes/admin-api/feature.js index a7f565efc8..954f8c2938 100644 --- a/lib/routes/admin-api/feature.js +++ b/lib/routes/admin-api/feature.js @@ -108,7 +108,7 @@ module.exports.router = function(config) { router.post('/validate', (req, res) => { req.checkBody('name', 'Name is required').notEmpty(); - req.checkBody('name', 'Name must be URL friendly').isUrlFirendlyName(); + req.checkBody('name', 'Name must be URL friendly').isUrlFriendlyName(); validateRequest(req) .then(validateUniqueName) @@ -118,7 +118,7 @@ module.exports.router = function(config) { router.post('/', (req, res) => { req.checkBody('name', 'Name is required').notEmpty(); - req.checkBody('name', 'Name must be URL friendly').isUrlFirendlyName(); + req.checkBody('name', 'Name must be URL friendly').isUrlFriendlyName(); const userName = extractUser(req);