From e1c0c5ce986061e6574858aed5d175d34e07cf21 Mon Sep 17 00:00:00 2001 From: Sebastian Gierlinger Date: Mon, 28 Jul 2014 15:19:49 +0200 Subject: [PATCH] Change refresh token expiry no issue - acquiring a new access token using a refresh token sets the expiration time of the refresh token to now + 24 hrs. - moved all occurrences of ONE_HOUR, ONE_DAY and ONE_YEAR to `core/server/utils` --- core/server/api/authentication.js | 4 +- core/server/api/users.js | 3 +- core/server/middleware/index.js | 60 +++++++++----------- core/server/middleware/middleware.js | 5 +- core/server/middleware/oauth.js | 20 +++++-- core/server/routes/admin.js | 8 +-- core/server/routes/frontend.js | 6 +- core/server/storage/localfilesystem.js | 24 ++++---- core/server/utils/index.js | 10 ++++ core/test/functional/routes/admin_test.js | 6 +- core/test/functional/routes/frontend_test.js | 6 +- core/test/utils/index.js | 4 +- 12 files changed, 80 insertions(+), 76 deletions(-) diff --git a/core/server/api/authentication.js b/core/server/api/authentication.js index 02cd5a12da..d9e6193c75 100644 --- a/core/server/api/authentication.js +++ b/core/server/api/authentication.js @@ -2,11 +2,11 @@ var _ = require('lodash'), dataProvider = require('../models'), settings = require('./settings'), mail = require('./mail'), + globalUtils = require('../utils'), utils = require('./utils'), when = require('when'), errors = require('../errors'), config = require('../config'), - ONE_DAY = 60 * 60 * 24 * 1000, authentication; /** @@ -23,7 +23,7 @@ authentication = { * @returns {Promise(passwordreset)} message */ generateResetToken: function generateResetToken(object) { - var expires = Date.now() + ONE_DAY, + var expires = Date.now() + globalUtils.ONE_DAY_MS, email; return authentication.isSetup().then(function (result) { diff --git a/core/server/api/users.js b/core/server/api/users.js index 77e80c2271..dfbc400a77 100644 --- a/core/server/api/users.js +++ b/core/server/api/users.js @@ -12,7 +12,6 @@ var when = require('when'), mail = require('./mail'), docName = 'users', - ONE_DAY = 60 * 60 * 24 * 1000, // TODO: implement created_by, updated_by allowedIncludes = ['permissions', 'roles', 'roles.permissions'], users; @@ -179,7 +178,7 @@ users = { user = invitedUser.toJSON(); return settings.read({context: {internal: true}, key: 'dbHash'}); }).then(function (response) { - var expires = Date.now() + (14 * ONE_DAY), + var expires = Date.now() + (14 * globalUtils.ONE_DAY_MS), dbHash = response.settings[0].value; return dataProvider.User.generateResetToken(user.email, expires, dbHash); }).then(function (resetToken) { diff --git a/core/server/middleware/index.js b/core/server/middleware/index.js index ee4862347d..bb94e3f9cd 100644 --- a/core/server/middleware/index.js +++ b/core/server/middleware/index.js @@ -2,35 +2,31 @@ // The following custom middleware functions cannot yet be unit tested, and as such are kept separate from // the testable custom middleware functions in middleware.js -var api = require('../api'), - bodyParser = require('body-parser'), - config = require('../config'), - errors = require('../errors'), - express = require('express'), - favicon = require('static-favicon'), - fs = require('fs'), - hbs = require('express-hbs'), - logger = require('morgan'), - middleware = require('./middleware'), - packageInfo = require('../../../package.json'), - path = require('path'), - routes = require('../routes'), - slashes = require('connect-slashes'), - storage = require('../storage'), - url = require('url'), - _ = require('lodash'), - passport = require('passport'), - oauth = require('./oauth'), - oauth2orize = require('oauth2orize'), +var api = require('../api'), + bodyParser = require('body-parser'), + config = require('../config'), + errors = require('../errors'), + express = require('express'), + favicon = require('static-favicon'), + fs = require('fs'), + hbs = require('express-hbs'), + logger = require('morgan'), + middleware = require('./middleware'), + packageInfo = require('../../../package.json'), + path = require('path'), + routes = require('../routes'), + slashes = require('connect-slashes'), + storage = require('../storage'), + url = require('url'), + _ = require('lodash'), + passport = require('passport'), + oauth = require('./oauth'), + oauth2orize = require('oauth2orize'), authStrategies = require('./authStrategies'), + utils = require('../utils'), expressServer, - setupMiddleware, - - ONE_HOUR_S = 60 * 60, - ONE_YEAR_S = 365 * 24 * ONE_HOUR_S, - ONE_HOUR_MS = ONE_HOUR_S * 1000, - ONE_YEAR_MS = 365 * 24 * ONE_HOUR_MS; + setupMiddleware; // ##Custom Middleware @@ -209,7 +205,7 @@ function robots() { headers: { 'Content-Type': 'text/plain', 'Content-Length': buf.length, - 'Cache-Control': 'public, max-age=' + ONE_YEAR_MS / 1000 + 'Cache-Control': 'public, max-age=' + utils.ONE_YEAR_S }, body: buf }; @@ -255,17 +251,17 @@ setupMiddleware = function (server) { expressServer.use(subdir, favicon(corePath + '/shared/favicon.ico')); // Static assets - expressServer.use(subdir + '/shared', express['static'](path.join(corePath, '/shared'), {maxAge: ONE_HOUR_MS})); + expressServer.use(subdir + '/shared', express['static'](path.join(corePath, '/shared'), {maxAge: utils.ONE_HOUR_MS})); expressServer.use(subdir + '/content/images', storage.get_storage().serve()); - expressServer.use(subdir + '/ghost/scripts', express['static'](path.join(corePath, '/built/scripts'), {maxAge: ONE_YEAR_MS})); - expressServer.use(subdir + '/public', express['static'](path.join(corePath, '/built/public'), {maxAge: ONE_YEAR_MS})); + expressServer.use(subdir + '/ghost/scripts', express['static'](path.join(corePath, '/built/scripts'), {maxAge: utils.ONE_YEAR_MS})); + expressServer.use(subdir + '/public', express['static'](path.join(corePath, '/built/public'), {maxAge: utils.ONE_YEAR_MS})); // First determine whether we're serving admin or theme content expressServer.use(updateActiveTheme); expressServer.use(decideContext); // Admin only config - expressServer.use(subdir + '/ghost', middleware.whenEnabled('admin', express['static'](path.join(corePath, '/client/assets'), {maxAge: ONE_YEAR_MS}))); + expressServer.use(subdir + '/ghost', middleware.whenEnabled('admin', express['static'](path.join(corePath, '/client/assets'), {maxAge: utils.ONE_YEAR_MS}))); // Force SSL // NOTE: Importantly this is _after_ the check above for admin-theme static resources, @@ -280,7 +276,7 @@ setupMiddleware = function (server) { expressServer.use(robots()); // Add in all trailing slashes - expressServer.use(slashes(true, {headers: {'Cache-Control': 'public, max-age=' + ONE_YEAR_S}})); + expressServer.use(slashes(true, {headers: {'Cache-Control': 'public, max-age=' + utils.ONE_YEAR_S}})); // Body parsing expressServer.use(bodyParser.json()); diff --git a/core/server/middleware/middleware.js b/core/server/middleware/middleware.js index ee8dcec14c..4f27cd99e2 100644 --- a/core/server/middleware/middleware.js +++ b/core/server/middleware/middleware.js @@ -10,11 +10,10 @@ var _ = require('lodash'), api = require('../api'), passport = require('passport'), errors = require('../errors'), + utils = require('../utils'), expressServer, oauthServer, - ONE_HOUR_MS = 60 * 60 * 1000, - ONE_YEAR_MS = 365 * 24 * ONE_HOUR_MS, loginSecurity = []; function isBlackListedFileType(file) { @@ -130,7 +129,7 @@ var middleware = { api.settings.read({context: {internal: true}, key: 'activeTheme'}).then(function (response) { var activeTheme = response.settings[0]; - express['static'](path.join(config.paths.themePath, activeTheme.value), {maxAge: ONE_YEAR_MS})(req, res, next); + express['static'](path.join(config.paths.themePath, activeTheme.value), {maxAge: utils.ONE_YEAR_MS})(req, res, next); }); }, diff --git a/core/server/middleware/oauth.js b/core/server/middleware/oauth.js index c97dd48d5e..309b48957d 100644 --- a/core/server/middleware/oauth.js +++ b/core/server/middleware/oauth.js @@ -33,13 +33,13 @@ oauth = { //Everything validated, return the access- and refreshtoken var accessToken = utils.uid(256), refreshToken = utils.uid(256), - accessExpires = Date.now() + 3600 * 1000, - refreshExpires = Date.now() + 3600 * 24 * 1000; + accessExpires = Date.now() + utils.ONE_HOUR_MS, + refreshExpires = Date.now() + utils.ONE_DAY_MS; return models.Accesstoken.add({token: accessToken, user_id: user.id, client_id: client.id, expires: accessExpires}).then(function () { return models.Refreshtoken.add({token: refreshToken, user_id: user.id, client_id: client.id, expires: refreshExpires}); }).then(function () { - return done(null, accessToken, refreshToken, {expires_in: 3600}); + return done(null, accessToken, refreshToken, {expires_in: utils.ONE_HOUR_S}); }).catch(function () { return done(null, false); }); @@ -62,11 +62,19 @@ oauth = { } else { var token = model.toJSON(), accessToken = utils.uid(256), - accessExpires = Date.now() + 3600 * 1000; + accessExpires = Date.now() + utils.ONE_HOUR_MS, + refreshExpires = Date.now() + utils.ONE_DAY_MS; if (token.expires > Date.now()) { - models.Accesstoken.add({token: accessToken, user_id: token.user_id, client_id: token.client_id, expires: accessExpires}).then(function () { - return done(null, accessToken); + models.Accesstoken.add({ + token: accessToken, + user_id: token.user_id, + client_id: token.client_id, + expires: accessExpires + }).then(function () { + return models.Refreshtoken.edit({expires: refreshExpires}, {id: token.id}); + }).then(function () { + return done(null, accessToken, {expires_in: utils.ONE_HOUR_S}); }).catch(function () { return done(null, false); }); diff --git a/core/server/routes/admin.js b/core/server/routes/admin.js index e0fff8f58a..209df77213 100644 --- a/core/server/routes/admin.js +++ b/core/server/routes/admin.js @@ -1,9 +1,7 @@ var admin = require('../controllers/admin'), config = require('../config'), express = require('express'), - - ONE_HOUR_S = 60 * 60, - ONE_YEAR_S = 365 * 24 * ONE_HOUR_S, + utils = require('../utils'), adminRoutes; @@ -14,12 +12,12 @@ adminRoutes = function (middleware) { // ### Admin routes router.get(/^\/(logout|signout)\/$/, function redirect(req, res) { /*jslint unparam:true*/ - res.set({'Cache-Control': 'public, max-age=' + ONE_YEAR_S}); + res.set({'Cache-Control': 'public, max-age=' + utils.ONE_YEAR_S}); res.redirect(301, subdir + '/ghost/signout/'); }); router.get(/^\/signup\/$/, function redirect(req, res) { /*jslint unparam:true*/ - res.set({'Cache-Control': 'public, max-age=' + ONE_YEAR_S}); + res.set({'Cache-Control': 'public, max-age=' + utils.ONE_YEAR_S}); res.redirect(301, subdir + '/ghost/signup/'); }); diff --git a/core/server/routes/frontend.js b/core/server/routes/frontend.js index 0a86f9dfb4..24046eac52 100644 --- a/core/server/routes/frontend.js +++ b/core/server/routes/frontend.js @@ -1,9 +1,7 @@ var frontend = require('../controllers/frontend'), config = require('../config'), express = require('express'), - - ONE_HOUR_S = 60 * 60, - ONE_YEAR_S = 365 * 24 * ONE_HOUR_S, + utils = require('../utils'), frontendRoutes; @@ -16,7 +14,7 @@ frontendRoutes = function () { router.get('/rss/:page/', frontend.rss); router.get('/feed/', function redirect(req, res) { /*jshint unused:true*/ - res.set({'Cache-Control': 'public, max-age=' + ONE_YEAR_S}); + res.set({'Cache-Control': 'public, max-age=' + utils.ONE_YEAR_S}); res.redirect(301, subdir + '/rss/'); }); diff --git a/core/server/storage/localfilesystem.js b/core/server/storage/localfilesystem.js index e227a645ff..0abd1228f4 100644 --- a/core/server/storage/localfilesystem.js +++ b/core/server/storage/localfilesystem.js @@ -1,15 +1,16 @@ // # Local File System Image Storage module // The (default) module for storing images, using the local file system -var _ = require('lodash'), - express = require('express'), - fs = require('fs-extra'), - nodefn = require('when/node'), - path = require('path'), - when = require('when'), - errors = require('../errors'), - config = require('../config'), - baseStore = require('./base'), +var _ = require('lodash'), + express = require('express'), + fs = require('fs-extra'), + nodefn = require('when/node'), + path = require('path'), + when = require('when'), + errors = require('../errors'), + config = require('../config'), + utils = require('../utils'), + baseStore = require('./base'), localFileStore; @@ -54,11 +55,8 @@ localFileStore = _.extend(baseStore, { // middleware for serving the files 'serve': function () { - var ONE_HOUR_MS = 60 * 60 * 1000, - ONE_YEAR_MS = 365 * 24 * ONE_HOUR_MS; - // For some reason send divides the max age number by 1000 - return express['static'](config.paths.imagesPath, {maxAge: ONE_YEAR_MS}); + return express['static'](config.paths.imagesPath, {maxAge: utils.ONE_YEAR_MS}); } }); diff --git a/core/server/utils/index.js b/core/server/utils/index.js index 579eb25b2b..a8c1add796 100644 --- a/core/server/utils/index.js +++ b/core/server/utils/index.js @@ -16,6 +16,16 @@ getRandomInt = function (min, max) { }; utils = { + /** + * Timespans in seconds and milliseconds for better readability + */ + ONE_HOUR_S: 3600, + ONE_DAY_S: 86400, + ONE_YEAR_S: 31536000, + ONE_HOUR_MS: 3600000, + ONE_DAY_MS: 86400000, + ONE_YEAR_MS: 31536000000, + /** * Return a unique identifier with the given `len`. * diff --git a/core/test/functional/routes/admin_test.js b/core/test/functional/routes/admin_test.js index 111583a135..b62620a9c9 100644 --- a/core/test/functional/routes/admin_test.js +++ b/core/test/functional/routes/admin_test.js @@ -15,12 +15,10 @@ var request = require('supertest'), httpServer, agent = request.agent, - ONE_HOUR_S = 60 * 60, - ONE_YEAR_S = 365 * 24 * ONE_HOUR_S, cacheRules = { 'public': 'public, max-age=0', - 'hour': 'public, max-age=' + ONE_HOUR_S, - 'year': 'public, max-age=' + ONE_YEAR_S, + 'hour': 'public, max-age=' + testUtils.ONE_HOUR_S, + 'year': 'public, max-age=' + testUtils.ONE_YEAR_S, 'private': 'no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0' }; diff --git a/core/test/functional/routes/frontend_test.js b/core/test/functional/routes/frontend_test.js index 5a6addd8d1..d537350087 100644 --- a/core/test/functional/routes/frontend_test.js +++ b/core/test/functional/routes/frontend_test.js @@ -15,12 +15,10 @@ var request = require('supertest'), ghost = require('../../../../core'), httpServer, - ONE_HOUR_S = 60 * 60, - ONE_YEAR_S = 365 * 24 * ONE_HOUR_S, cacheRules = { 'public': 'public, max-age=0', - 'hour': 'public, max-age=' + ONE_HOUR_S, - 'year': 'public, max-age=' + ONE_YEAR_S, + 'hour': 'public, max-age=' + testUtils.ONE_HOUR_S, + 'year': 'public, max-age=' + testUtils.ONE_YEAR_S, 'private': 'no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0' }; diff --git a/core/test/utils/index.js b/core/test/utils/index.js index 6650166f3d..5438bc9611 100644 --- a/core/test/utils/index.js +++ b/core/test/utils/index.js @@ -424,5 +424,7 @@ module.exports = { editor: 2, author: 3 } - } + }, + ONE_HOUR_S: 3600, + ONE_YEAR_S: 31536000 };